Ships not moving on water

ID:2000150
 
Dec 18 2015, 4:17 pm
(See the best response by Ter13.)
Code:
turf
  deep_water
    density = 1
    Enter(var/atom/movable/O)
      if(istype(O,/obj/boat))
        for(var/atom/movable/B in src)
          if(B.density)
            return 0
        return 1
      else
        return ..(O)
mob
  var
    obj/vehicle

client
  Move(newloc,var/ndir)
    if(src.mob.vehicle)
      return src.mob.vehicle.Move(newloc,ndir)
    else
      return src.mob.Move(newloc,ndir)
obj
  boat
    icon='Boat.dmi'
    var
      mob/pilot
    density = 1
    verb
      pilot()
        if(!src.pilot)
          src.pilot = usr
          usr.vehicle = src
        else
          usr << "There is already a pilot!"
      board()
        set src in oview(1)
        if(src.contents.len<4)
          if(!src.pilot)
            src.pilot = usr
            usr.vehicle = src
          src.contents += usr
      unboard() //I don't forgot the word.
        set src in oview(1)
        src.loc = usr.loc
        usr.loc = src.loc


Problem description:
This was actually a code I found on the forums, when I attempted to implant it however I was unable to figure this out. In world the boat/ship does not move from its location at all. It will turn with the keys but never move. Would anyone mind teaching/helping me with the ship code?

Dec 18 2015, 7:13 pm
Check out the value of newloc inside of client/Move(). Since the client's mob is inside of the vehicle, the next step in any direction will be a null location. This will always result in movement failure.

I played around with your idea a bit.

For starters, I really hate the overriding Enter() to allow specific objects through. IMO, this kind of a thing would be best done with a general approach that can be recycled for different objects later on.

The reason that programming specific turfs to react to specific objects is bad is because you are telling the water to tell the boat what it can do, rather than the boat deciding what it will do. With OOP, it's usually best to let the objects handle their own behavior. Entering water is a mutual behavior of water and boat, but IMO, water itself shouldn't have to check that a specific type of object is entering it unless the behavior is exclusively confined to the object in question.

Let's imagine a game where you have different types of terrain and vehicles for bypassing that terrain. We want to allow ships to move on the ocean, rowboats to move on rivers, people to move on regular ground, forests, and hills, a horse to be able to move on ground and forests, a wagon to move on regular ground only, and we want an airship that can move wherever it damn pleases.

With this approach, you'd have to write a system that would account for each type of vehicle with each type of tile.

Instead, we're going to do something much, much simpler.

//define some bitflags for the terrain system:

#define TERRAIN_GROUND    1
#define TERRAIN_HILL      2
#define TERRAIN_FOREST    4
#define TERRAIN_RIVER     8
#define TERRAIN_MOUNTAIN 16
#define TERRAIN_OCEAN    32
#define TERRAIN_INDOOR   64

#define TERRAIN_ALL      127 //the result of all the above values added together
#define TERRAIN_WATER    40 //the result of river and ocean together
#define TERRAIN_PASSABLE 71 //the result of ground, hill, indoor, and forest added together

atom
    movable
        var
            //defines what tiles this atom collides with
            terrain_mask = 0

turf
    var
        //defines what kind of terrain this object is
        terrain_flags = TERRAIN_GROUND

    Enter(atom/movable/o)
        return ..()&&(!terrain_flags&o.terrain_mask) //use binary logic to allow a normal move plus any move that contains a permitted terrain flag


Basically, all we did here was add a single variable to /atom/movable and /turf each which when used together determine what kind of terrain this movable atom can walk on. When we "&" (binary and) the two values together, it returns a set of the individual flags that happened to match added together.

Let's set up our turfs, player and vehicles:

turf
    terrain_flags = TERRAIN_GROUND
    forest
        terrain_flags = TERRAIN_FOREST
    hill
        terrain_flags = TERRAIN_HILL
    mountain
        terrain_flags = TERRAIN_MOUNTAIN
    river
        terrain_flags = TERRAIN_RIVER
    ocean
        terrain_flags = TERRAIN_OCEAN
mob
    terrain_mask = TERRAIN_ALL^TERRAIN_PASSABLE //blocked by everything but the passable flags

obj
    vehicle
        density = 1
        horse
            terrain_mask = TERRAIN_WATER|TERRAIN_HILL|TERRAIN_MOUNTAIN|TERRAIN_INDOOR //blocked by water, hills, and mountains. Also, horses aren't allowed inside because that would be silly.
        wagon
            terrain_mask = TERRAIN_WATER|TERRAIN_FOREST|TERRAIN_HILL|TERRAIN_MOUNTAIN|TERRAIN_INDOOR //blocked by water, forest, hills, and mountains. Wagons indoors involves an insurance claim.
        rowboat
            terrain_mask = TERRAIN_ALL^TERRAIN_RIVER //blocked by everything but river
        ship
            terrain_mask = TERRAIN_ALL^TERRAIN_OCEAN //blocked by everything but ocean
        airship
            terrain_mask = 0 //blocked by nothing


That should about do it for handling turf density.

I'll continue this in part 2 for handling vehicle movement and fixing your actual issue.
Dec 18 2015, 7:14 pm
Kaiochao wrote:
Your boat is the dense movable in Enter.

Nope.
Dec 18 2015, 7:37 pm (Edited on Dec 18 2015, 7:52 pm)
Best response
As for handling the vehicle movement... I'd like to go over some utility functions and definitions that you will find helpful.

First, utility definitions:

#define clamp(V,L,H) min(max(V,L),H)
#define TILE_WIDTH 32
#define TILE_HEIGHT 32


clamp() is a shortcut that allows you to clamp values between a low and a high value. That means that the value V supplied will never be less than L or more than H.

TILE_WIDTH and TILE_HEIGHT I include in almost all my code snippets because not everyone works at the same tile size, so it's useful for showing the math of working with tile/pixel games without using too specific values.

Some utility functions:

sorted_nearby_tiles(atom/ref)
        if(!ref.z) return list()
        var/d = ref.dir
        return list(get_step(ref,d),get_step(ref,turn(d,45)),get_step(ref,turn(d,-45)),get_step(ref,turn(d,90)),get_step(ref,turn(d,-90)),get_step(ref,turn(d,135)),get_step(ref,turn(d,-135)),get_step(ref,turn(d,180)))


This function returns a list of tiles nearby something sorted by priority of direction. This will be useful later. Don't worry about it for now.

atom
    movable
        var
            terrain_mask = 0
        proc
            //like Move() but it can't fail. It won't call Enter()/Exit()/Cross()/Uncross(), only Entered()/Exited()/Crossed()/Uncrossed()
            ForceMove(atom/newloc,Dir=0,Step_x=0,Step_y=0)
                var/oloc = loc
                var/list/al = list()
                var/list/tl = istype(loc,/turf) ? locs : loc ? list(loc) : list()
                var/list/ol = obounds(src)

                loc = newloc
                if(Dir) dir = Dir
                step_x = Step_x
                step_y = Step_y

                var/list/an = list()
                var/list/tn = istype(loc,/turf) ? locs : loc ? list(loc) : list()
                var/list/on = obounds(src)

                for(var/turf/t in tl)
                    al |= t.loc
                for(var/turf/t in tn)
                    an |= t.loc

                var/list/xa = al-an
                var/list/xl = tl-tn
                var/list/xo = ol-on

                var/list/na = an-al
                var/list/nl = tn-tl
                var/list/no = on-ol

                for(var/atom/movable/o in xo)
                    o.Uncrossed(src)
                for(var/atom/o in xl)
                    o.Exited(src,loc)
                for(var/area/a in xa)
                    a.Exited(src,loc)

                for(var/area/a in na)
                    a.Entered(src,oloc)
                for(var/atom/o in nl)
                    o.Entered(src,oloc)
                for(var/atom/movable/o in no)
                    o.Crossed(src)

            Step(Dir=0,Step_size=0)
                if(!Step_size) Step_size = step_size
                if(!Dir) Dir = dir
                if(isturf(loc))
                    var/dx = step_x
                    var/dy = step_y
                    if(Dir&NORTH)
                        dy += Step_size||TILE_HEIGHT
                    else if(Dir&SOUTH)
                        dy -= Step_size||TILE_HEIGHT
                    if(Dir&EAST)
                        dx += Step_size||TILE_WIDTH
                    else if(Dir&WEST)
                        dx -= Step_size||TILE_WIDTH
                    dx = clamp(dx+(x-1)*TILE_WIDTH,0,world.maxx*32-1)
                    dy = clamp(dy+(y-1)*TILE_HEIGHT,0,world.maxy*32-1)
                    return Move(locate(dx/TILE_WIDTH+1,dy/TILE_HEIGHT+1,z),Dir,dx%TILE_WIDTH,dy%TILE_WIDTH)
                else
                    return Move(loc,Dir,step_x,step_y)
            Del()
                if(loc) ForceMove(null)
                ..()


atom/movable/proc/ForceMove() is a function that forces a movement to happen even if the conditions of Enter()/Exit() would normally fail. You should ALWAYS use Move() or ForceMove() to relocate something. This will ensure that you can rely on Entered()/Exited() being called, even if the object is being deleted.

atom/movable/proc/Step() is a function that's analogous to /proc/step(), but doesn't return the wrong values unlike /proc/step(), LUMMOX! This is an annoyance I have with the step() function that unfortunately will never be properly resolved because Lummox/Tom always argued that the documentation says that step() should return 1 or 0. However, Move() returns 1, 0, or the number of pixels actually moved depending on a number of factors. I created this function a while back just so I could fix a problem with the DM language that has been neglected for a long, long time.

With these in tow, we can go ahead and fix up a few things with your vehicle approach:

mob
        var
            obj/vehicle/vehicle

    Move(atom/newloc,Dir=0,Step_x=0,Step_y=0)
        return vehicle ? vehicle.Step(Dir) : ..()

    Logout()
        ForceMove(null)
        ..()
obj
    vehicle
        var
            mob/pilot
            list/passengers
            max_passengers = 0
        verb
            pilot()
                set src in oview(1)
                if(!pilot&&(get_dist(usr,src)<=1))
                    pilot = usr
                    usr.vehicle = src
                    verbs -= /obj/vehicle/verb/pilot
                    usr.ForceMove(src)
                    usr << "You take the helm!"
                    if(passengers&&(usr in passengers))
                        passengers -= usr
                        if(max_passengers==passengers.len+1)
                            verbs += /obj/vehicle/verb/embark
                        if(!passengers.len) passengers = null
                        else passengers << "[usr] takes the helm!"
                else if(pilot!=usr)
                    usr << "[pilot] is at the helm!"

            embark()
                set src in oview(1)
                if(usr.loc!=src)
                    if((!passengers&&max_passengers||passengers&&passengers.len<max_passengers)&&get_dist(usr,src)<=1)
                        usr.ForceMove(src)
                        if(!passengers) passengers = list(usr)
                        else
                            (passengers + pilot) << "[usr] boards [src]!"
                            passengers += usr

            disembark()
                set src = usr.loc
                usr << "herpyderp"
                var/list/l = sorted_nearby_tiles(src) + loc

                for(var/turf/t in l)
                    if(usr.Move(t))
                        return
                usr << "You cannot disembark here!"

        Exited(mob/m)
            ..()
            if(istype(m))
                if(m==pilot)
                    pilot = null
                    verbs += /obj/vehicle/verb/pilot
                    if(m.vehicle==src)
                        m.vehicle = null
                    passengers << "[m] leaves the helm!"
                else if(passengers&&(m in passengers))
                    passengers -= m
                    if(max_passengers==passengers.len+1)
                        verbs += /obj/vehicle/verb/embark
                    if(!passengers.len)
                        passengers = null
                    else
                        (passengers + pilot) << "[m] disembarks!"

        New()
            ..()
            if(!max_passengers)
                verbs -= /obj/vehicle/verb/embark


The major things I've changed here, are that I've fixed some safety problems with verbs. Sometimes a player can trigger a verb and it will make it to the server much later than it was clicked.

I also fixed a problem where if a player logged out while piloting a ship, the circular reference would never be cleaned up and the player would never surrender the helm of the ship, stranding any passengers potentially in the middle of the ocean.

I also fixed a problem where if a player was a passenger they could potentially never take over a ship that had been abandoned without exiting it first.

I also fixed another issue where if a player left a ship, they could wind up stranded in the middle of the ocean. The new disembark behavior actually fixes that by checking to make sure the player can actually exit the ship to a valid tile nearby. Of course, it's still possible to be marooned on a small island, but they'll be fine as long as there is rum?

Lastly, I embedded much of the passenger management into the Exited() function rather than in the verbs themselves. I also fixed some bad practices with relation to how you were managing locations and whatnot.


And of course, I changed the way that your movement hooks are actually being called. Instead of embedding the vehicle check in the client, I embedded it in the mob itself. It's much more robust, safer, and should actually allow NPCs to drive boats too without you having to write special behavior. Oh, and it actually works. Cheers!

Do take note of the max_passengers variable for vehicles. Just set it to the max number of passengers (remember the pilot isn't a passenger!) in order to make it work. Polymorphism is fun!
Dec 18 2015, 8:10 pm
In response to Ter13
Ter13 wrote:
> #define clamp(V,L,H) min(max(V,L),H)
> #define TILE_WIDTH 32
> #define TILE_HEIGHT 32
>

TILE_WIDTH and TILE_HEIGHT I include in almost all my code snippets because not everyone works at the same tile size, so it's useful for showing the math of working with tile/pixel games without using too specific values.

Fantastic post, but could you elaborate a little here, why is it better to use these defines than to use world.icon_size? Functionally they seem the same to me, but one is possibly innacurate.
Dec 18 2015, 8:22 pm
Accessing defines has less overhead than accessing a variable. A #define is basically just swapped out in the compiled code so there's no lookup at all, but when you use world.icon_size you're looking that variable's value up every time you do a calculation.