Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TD][RA] Two units in the same spot at the same time bugs #850

Open
ChthonVII opened this issue Oct 28, 2022 · 0 comments
Open

[TD][RA] Two units in the same spot at the same time bugs #850

ChthonVII opened this issue Oct 28, 2022 · 0 comments

Comments

@ChthonVII
Copy link
Contributor

There are a number of somewhat interrelated bugs in the movement system that can result in two units occupying the same spot at the same time.

  1. Two units moving at perpendicular diagonals may both occupy the shared corner coordinate (or coordinates very near it) at the same time. The cause is that only the destination cell is checked with Can_Enter_Cell() and what's happening on the sides of a diagonal move isn't considered.
  2. The track-jumping logic in DriveClass::While_Moving() changes the TrackNumber before calling Stop_Driver(). This causes Mark_Track() to get called with the combination of the old HeadTo and new TrackNumber -- which may correspond to cells that have nothing to do with this unit at all. Consequently, we may have one unit removing the cell reservation flags (Flag.Occupy.Vehicle) that a different unit set. This may cause a third unit to believe the formerly reserved cell is open, thus causing two units to drive into the cell at the same time.
  3. Some other quirk of the track system also permits one unit to remove a cell reservation flag that a different unit set via Mark(MARK_UP) and ultimately CellClass::Occupy_Up(), leading to the same results. I haven't verified this, but my suspicion is that the root cause has to do with off-by-one errors when rotating and transposing tracks. A coordinate offset -128 leptons from the center of a cell is within that same cell, but an offset +128 leptons is in the next cell. The rotation/transposition logic doesn't appear to account for this. So there are probably some possible tracks with some nodes that lie just over the line into another cell. This means that a unit will briefly pass through the edge/corner of a cell that someone else may have reserved, and will pull up the other unit's reservation flag when they exit it. That's my best theory anyway.
  4. When a unit is moving along a two-cell track, it does not recheck that the endpoint cell is still free when it reaches the per-cell-process point of the midpoint cell. Instead it just blithely drives on ahead. Fixing # 2 and # 3 should prevent another unit from occupying the cell, but # 1 remains a problem.
  5. When a unit does a track jump, it does not recheck that the new midpoint cell (which is the old endpoint cell) is still free. Fixing # 4 (which entails fixing # 1, # 2, and # 3) should fix this.
  6. When a unit calls Can_Enter_Cell() on a repair bay, it immediately returns MOVE_OK if the radio system says it can dock, without checking if there's anything else in the cell that should block it (e.g. the prior unit that got stuck there for some reason).

Fixes are as follows:

  1. Can_Enter_Cell() needs to check the diagonals. Seems simple enough, but the devil's in the details on this one.
    • The basic check is to look at the two diagonal cells and see if something occupying one has its HeadTo in the other.
    • In order to deal with slow-moving units that may have just barely passed the shared corner coordinate and will still be visually overlapping it when we get there, we also need to look for a unit with its HeadTo in the same cell that it's occupying and its Coord is within the box with corners at the shared corner coordinate and its HeadTo.
    • I haven't fully thought it through, but I believe fixing # 4 prevents the possibility of a fast-moving unit that's more than a cell away crossing through this unit.
    • If the quirks of the track system (see theory in # 3 above) put the other unit in the destination cell at the moment Can_Enter_Cell() is called, it will be treated as a blocker. (So no problems.)
    • I've not provided for the case where the quirks of the track system put the other unit in cell this unit plans to be moving out of at the moment Can_Enter_Cell is called. This seems to be rare enough that it's not worth the performance cost to deal with.
    • Blockers need to be separated by allegiance, since you may need to return MOVE_CLOAK. (Although you'll wind up shimmering the wrong cell.)
    • There's not a good answer for what to do if this unit is a crusher and the other unit is an enemy infantry. There's no way to perform a crush here, but it seems weird to allow blocking. I decided to treat it as MOVE_OK and hope it looks like a narrow escape from a crush.
    • There's not a good answer for what to do if both units are infantry belonging to the same house. Two lone infantry blocking would seem weird, but two groups of 5 walking through each other would also look weird. I decided to tally up the reserved infantry positions in all 4 cells and permit movement if the total is 5 or less.
    • If the other unit is an allied infantry, you need to continue checking other objects, because the other cell might have an enemy vehicle coming in the opposite direction to crush the infantry.
  2. Move Stop_Driver() up above the change of TrackNumber. (This fixes a huge fraction of the "two vehicles in one cell" occurrences!)
  3. Cells need a new DriveClass* member that stores which vehicle set the Flag.Occupy.Vehicle flag. CellClass::Occupy_Up() and Occupy_Down() and DriveClass::Mark_Track() need to set/clear this accordingly and prohibit anyone else from changing the flag.
  4. Add a call to Can_Enter_Cell() to the block in Drive_Class::While_Moving() that handles the per-cell-process point. Can_Enter_Cell() needs to be modified to ignore Flag.Occupy.Vehicle if it was set by this unit itself. (See fix for # 3 above). If the cell is blocked, we need to do a full stop and repath. (We can't just wait a tick because that would call Per_Cell_Process repeatedly. We need to repath because we previously ate Path[0] and now we're aborting the associated move.)
  5. I believe fixing # 4 should fix this too.
  6. Rather than return MOVE_OK as soon as you find a consenting repair bay, set a flag and continue with the while loop checking other objects in the cell. Immediately after the while loop, check if the flag is set and the pending result is still MOVE_OK, and potentially return MOVE_OK then.

Here is a commit on CFE Patch Redux containing most of these fixes.
Here is a later commit commenting out the fix for # 5 that, on second thought, isn't needed.
Here is the fix for repair bays. (Sorry it's buried with a bunch of other stuff. Look at UnitClass::Can_Enter_Cell().)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant