ID:2382205
 
Resolved
In stage 2 of the movement overhaul, the Cross/Uncross/Crossed/Uncrossed procs now are used by all atoms, not just movables.

By default, turf.Enter() will call turf.Cross() instead of checking turf density, before calling Cross() for any tile-based contents. This should allow for cleaner implementations by overriding Cross() instead of Enter().

Likewise, the default turf.Exit() will now call turf.Uncross() and then try its tile-based contents.

And of course, turf.Entered() will call turf.Crossed() by default, and likewise for Exited/Uncrossed.
Applies to:DM Language
Status: Resolved (512.1438)

This issue has been resolved.
Currently, the default behavior of turf/Enter() is to determine whether something can enter a turf by first checking if both the entering object, and the turf are dense. If so, return 0. If not, Enter() loops over contents and looks for objects that fully cover the turf, calling Cross() on each. If any of these refuse entry, return 0. Otherwise, return 1.

Attempts to change this default behavior result in weirdness.

turf/allowowner
var
owner
Enter(atom/movable/o)
if("\ref[o]"==owner)
return 1
else
return 0


The above would mean that Cross() would never be called for anything in the tile at all. This means that the owner could enter the turf, overlapping objects that should prevent them from overlapping that tile.

The correct way to do the above is:

turf/allowowner
var
owner
Enter(atom/movable/o)
return ..() && "\ref[o]"==owner


The other issue, is that this makes looping over a list returned by obounds() or the like unnecessarily difficult to test for collision:

for(var/atom/o in obounds(ref))
if(isturf(o))
if(!o.Enter(ref,ref.loc))
return 0
else if(!o.Cross(ref))
return 0
return 1


The other issue with this snippet, is the fact that Cross() will be called twice for some atoms, because that behavior is built into Enter().

Instead, I propose changing Enter()/Exit()/Entered()/Exit()'s default behavior:

Call src.Cross(o). If 0, eject early. If non-zero, search through full-tile blockages calling object.Cross(o), if zero, eject early. If exhausted, return 1.

Entered()'s default behavior should be to call turf.Crossed(o).

This change will result in looping over bounds lists much easier and saner:

for(var/atom/o in obounds(ref))
if(!o.Cross(ref))
return 0
return 1


No duplication problem here!

As well as not break any existing code, as existing projects will simply have either not used Cross() on a turf, or will have overridden Entered() without calling default behavior (because there was none), thus choosing to eliminate Crossed() and Uncrossed() calls from their project.

The other positive result, is that turf/Cross() and turf/Uncross() can be quick overrides that you don't need to know deeply how they work:

turf/allowowner
var
owner
Cross(atom/movable/o)
return "\ref[o]"==owner


The above code would prevent Cross() checks from being called on objects in the tile, which would be consistent with the intended way that object collision priorities should be sorted: turf first, full tile objects second, objects with lesser coverage third.
The other issue is built-in pathfinding. Built-in pathfinding does not consult Enter()/Exit()/Cross()/Uncross() it works purely off of density.

The problem with this, is that for any turf that you have overridden the default density rules, the turf actually needs to have a density of 0, or you have just destroyed the turf's ability to consider objects blockages and will have to completely rewrite the code to handle that by hand.

If the object does have a density of 0, though, that means that things using built-in pathfinding will try their damnedest to pathfind through the tile, even though they can't move into it at all, thus globally breaking built-in pathfinding for your project. (yay!)

This means that the object-checking behavior should happen in turf/Enter(), but the turf's density should be considered as the default action of turf/Cross():

//default action:

turf/Cross(atom/movable/o)
return o.density&&density

turf/Uncross(atom/movable/o)
return 1


Which means that instead of directly checking the density in Enter() to determine whether we check objects for Cross()/Uncross() behavior, we actually use turf/Cross() and turf/Uncross() to determine whether that loop happens or not.

Simple change, no project-breaking bullshit, huge quality of life improvements for everyone.


So the proposed global default behavior ought to look something like this:

atom
proc
Enter(atom/movable/o,atom/fromloc)
return 1

Exit(atom/movable/o,atom/toloc)
return 1

Cross(atom/movable/o)
return !(o.density && density)

Uncross(atom/movable/o)
return 1

Entered(atom/movable/o,atom/fromloc)
Exited(atom/movable/o,atom/toloc)
Crossed(atom/movable/o)
Uncrossed(atom/movable/o)

area
Enter(atom/movable/o,atom/fromloc)
return Cross(o)

Exit(atom/movable/o, atom/toloc)
return Uncross(o)

Entered(atom/movable/o,atom/fromloc)
Crossed(o)

Exited(atom/movable/o,atom/toloc)
Uncrossed(o)

Cross(atom/movable/o)
return 1

turf
Enter(atom/movable/o,atom/fromloc)
if(!Cross(o))
return 0
for(var/atom/movable/a in contents)
if(a.step_x + a.bound_x==0 && a.step_y + a.bound_y==0 && a.bound_width==TILE_WIDTH && a.bound_height==TILE_HEIGHT && !a.Cross(o))
return 0
return 1

Exit(atom/movable/o,atom/toloc)
if(!Uncross(o))
return 0
for(var/atom/movable/a in contents)
if(a.step_x + a.bound_x==0 && a.step_y + a.bound_y==0 && a.bound_width==TILE_WIDTH && a.bound_height==TILE_HEIGHT && !a.Uncross(o))
return 0
return 1

Entered(atom/movable/o,atom/fromloc)
Crossed(o)

Exited(atom/movable/o,atom/toloc)
Uncrossed(o)


Ideally, though, turf/Enter() would also handle area entry in Enter() to make certain that all "on the map" behavior is rooted on turfs, but that would involve complex checks in Enter() to make sure the player isn't in an area before testing turf.loc.Cross(), which is probably not desirable.
An addendum to this would be that step_x, step_y and bounds should be present on all atoms to bring them into parity, so we don't have to type-test something before calculating its pixel positions. for turfs, I do not propose allowing bounds to actually change the bounding area of the turf, or having step_x or step_y have any kind of an effect.

If that's not feasible, there should be a fast ismovable() type check added to the language that will sidestep the speed concerns of using istype(o,/atom/movable).
These are some good changes. It would have been nice to have this when I was doing some work on preping for conversion to pixel movement.
Lummox JR resolved issue with message:
In stage 2 of the movement overhaul, the Cross/Uncross/Crossed/Uncrossed procs now are used by all atoms, not just movables.

By default, turf.Enter() will call turf.Cross() instead of checking turf density, before calling Cross() for any tile-based contents. This should allow for cleaner implementations by overriding Cross() instead of Enter().

Likewise, the default turf.Exit() will now call turf.Uncross() and then try its tile-based contents.

And of course, turf.Entered() will call turf.Crossed() by default, and likewise for Exited/Uncrossed.