Units::teleport: keep occupancy flags while other units remain on the tile - #5946
Alistair-Afton wants to merge 2 commits into
Conversation
SilasD
left a comment
SilasD
left a comment
There was a problem hiding this comment.
this one is NOT ready for prime-time.
I think this will work 99% of the time, but there are some interesting special cases that should be considered and possibly resolved.
this one probably needs attention from ab9rf or Quietust.
| for (auto other : world->units.active) { | ||
| if (other == unit) | ||
| continue; | ||
| auto &tiles = other->flags1.bits.on_ground ? grounded_tiles : standing_tiles; |
There was a problem hiding this comment.
special cases that need consideration:
- are units that are flying standing, grounded, or neither?
-
- (I think they follow the normal rule of only one standing per tile, but I don't know that to be true.)
- are units that are swimming standing, grounded, or neither?
-
- (again I think the standard rule, but I don't know for certain.)
- are units that are projectiles standing, grounded, or neither?
-
- I'm guessing neither, but that's only a guess.
- are units that are in a built cage standing, grounded, or neither?
- are units that are in a loose cage standing, grounded, or neither?
-
- I think in both cage cases they're grounded or neither, because units can move through the cage's tile without crawling.
- Edit: what about ghostly units? standing, grounded, or neither?
There was a problem hiding this comment.
Checked these in a live fort:
- flying: there is no flying unit flag; fliers are ordinary units on the map and set occupancy normally
- swimming: verified
on_ground=falsewithocc.unit=trueon their tile -- swimming units count as standing - projectiles: now skipped in the recompute scan (
flags1.projectile) - caged (built and loose cages both use
flags1.caged): verified two caged units haveon_ground=falseand their pos tile has neither occupancy flag set -- caged units do not set occupancy, so they are now skipped in the scan - ghostly: none in the test fort, so I could not verify; they are left counted since I have no evidence they differ from normal units
| if occ then | ||
| if unit.flags1.on_ground then | ||
| occ.unit_grounded = true | ||
| else | ||
| occ.unit = true | ||
| end | ||
| end |
There was a problem hiding this comment.
standing vs. grounded is treated as a binary here, which is probably fine in a test suite.
Edit: presumably also considered a binary in the actual test code below. again, fine for a test suite.
| if occ then | ||
| occ.unit = false | ||
| occ.unit_grounded = false | ||
| end | ||
| end | ||
| for _, unit in ipairs(units) do | ||
| local occ = tile_occupancy(unit.pos) | ||
| if occ then | ||
| if unit.flags1.on_ground then | ||
| occ.unit_grounded = true | ||
| else | ||
| occ.unit = true | ||
| end |
There was a problem hiding this comment.
I don't think this works. occ is not a pointer to the tile occupancy, it is a copy of it.
we don't have a setter yet, so you need to use this after line 47 and line 56:
dfhack.maps.getTileBlock(pos).occupancy[pos.x%16][pos.y%16] = occ
(untested code)
substitute unit.pos for the line 56 copy.
you can skip a nil test, the tile is known to exist, so the block also exists.
There was a problem hiding this comment.
I checked this in-game: getTileFlags pushes the pointer returned by Maps::getTileOccupancy, which is &block->occupancy[x&15][y&15], so the Lua object is a live ref into the block (same address as getTileBlock(pos).occupancy[x%16][y%16]), and direct field writes do propagate. The mutation sites work as written; the only change needed was reading via getTileFlags, which I have now adopted.
Caged units and in-flight projectiles do not set tile occupancy flags, so they must not keep flags alive when other units leave a tile. Test now uses dfhack.maps.getTileFlags, requires the destination tile to be walkable, and returns a failure reason from free_tile_near.
2 participants
Summary
Fixes #5938.
Units::teleportcleared the source tile'sunit_groundedflag unconditionally, so teleporting one grounded unit away from a tile left the flag unset even when another grounded unit still occupied it. The game itself only clears the flag when the last grounded unit leaves the tile.teleport now records which tiles still have a grounded or standing unit on them (honoring the 3x3 EQUIPMENT footprint) and only clears
unit_grounded/unitwhen the departing unit was the last of its kind there.Testing
test/modules/units_fortress.luacovers both directions: teleporting one of two grounded units off a tile keepsunit_grounded, teleporting one of two standing units keepsunit, and removing the last unit still clears the flag.