beyond-all-reason / beyond-all-reason/RecoilEngine

`gadget:AllowUnitTransport` can technically desync

Open
#2,223 0 comments 0 reactions 1 assignee Claimed by @sprunk View on GitHub
Dominant language
C++
Stars
679
Forks
290
Avg merge
3d 2h
Merged PRs (30d)
40

Description

`AllowUnitTransport` is synced but can be called from unsynced, here's a "trace":

https://github.com/beyond-all-reason/RecoilEngine/blob/604f00d290a2e3ddbc2fae9f27f10b9c8efedc0a/rts/Lua/LuaHandleSynced.cpp#L818
https://github.com/beyond-all-reason/RecoilEngine/blob/604f00d290a2e3ddbc2fae9f27f10b9c8efedc0a/rts/Sim/Units/Unit.cpp#L2549-L2558
https://github.com/beyond-all-reason/RecoilEngine/blob/604f00d290a2e3ddbc2fae9f27f10b9c8efedc0a/rts/Sim/Units/CommandAI/MobileCAI.cpp#L937-L946
https://github.com/beyond-all-reason/RecoilEngine/blob/604f00d290a2e3ddbc2fae9f27f10b9c8efedc0a/rts/Game/SelectedUnitsHandler.cpp#L868-L900
https://github.com/beyond-all-reason/RecoilEngine/blob/604f00d290a2e3ddbc2fae9f27f10b9c8efedc0a/rts/Game/UI/GuiHandler.cpp#L1679-L1708

Reproduction:

* take https://github.com/ZeroK-RTS/Zero-K/tree/artificial-allowunittransport-desync (makes `AllowUnitTransport` reduce health)
* build a transport (e.g. `gunshiptrans`), select it (so that the load command is checked), hover over some units. Whatever you hover over something, it loses health
* watch the replay and keep your cursor away. Units don't lose health

This has been the case perhaps forever, so it most likely not related to the recent desyncs. ZK's real `gadget:AllowUnitTransport` adds units to a cache like this:
```lua
if allowTransportCache[transporterUnitDefID][transporteeUnitDefID] then
return (allowTransportCache[transporterUnitDefID][transporteeUnitDefID] == 1)
end
local allowed = not UnitDefs[transporteeUnitDefID].customParams.requireheavytrans
allowTransportCache[transporterUnitDefID][transporteeUnitDefID] = ((allowed and 1) or 0)
return allowed
```
This does not affect unit positions etc., only the contents of that `allowTransportCache` table which affects nothing downstream (since on a "real" call later it would get filled with the same value anyway), so I somewhat doubt that this can actually trigger the desync warning either. But the capability is there and IDK about other games.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.