Defer UI operations until UI system runs frame update

[eoineoineoin]

In the UI system, some updates are applied immediately, while some are deferred until the frame update. This changes all the operations to be deferred. This fixes a UI bug, but there's a more fundamental underlying bug here.

First, the bug;

1. forcemap Packed
2. startround
3. Find the R&D server, spawn some research point disks and add them to the server.
4. Open the UI of the R&D computer beside the server and press the "Research" button on one of the technologies. You'll see the UI update - available points decrease, get a new entry in "Unlocked technologies" and you get new technology options to research.
5. Close that UI.
6. Open the UI of one of the other two R&D computer and press "Research" on one of the technologies. You don't get a proper UI update; the points decrease, but you don't get a new entry in the "Unlocked technologies" nor new technology options to research.
7. Close the UI and open it again - it's now updated.

For clarification:

Two important components here; the TechnologyDatabaseComponent stores the information on unlocked and available technologies, and the UserInterfaceComponent provides the UI (uh-huh). The ResearchConsoleBoundUserInterface.UpdateState calls an update on the ResearchConsoleMenu, which in turn reads the TechnologyDatabaseComponent to populate the UI. When you press the "Research" button, a message is sent to the server which causes it to Dirty() both of these components.

For the broken computers, the ClientGameStateManager.ApplyGameState applies the state to the UserInterfaceComponent before the TechnologyDatabaseComponent, but because the SharedUserInterfaceSystem has a ComponentHandleState subscription, it performs the work in that callback immediately so the UI ends up reading stale values. By deferring all the UI operations until the frame render, we know that any components the UI wants to read will have had their state applied.

I didn't look into why one of the computers seems to work - suspect it's simply serialization and the order of components stored in that map.

There seems to be a more fundamental issue here; because the component state updates are unordered, any time you have a system which subscribes to ComponentHandleState and reads from a different component, you're in danger of getting stale data. I did a quick scan over all the subscriptions in content and most look pretty benign, though it's gonna be a bunch of work to untangle, especially with systems which raise further events in that subscription. SharedSolutionContainerSystem looks suspicious, for example, while FixtureSystem (in engine) seems to have a much clearer issue if the updates arrive out of order.

[metalgearsloth]

good stuff

[metalgearsloth]

There seems to be a more fundamental issue here; because the component state updates are unordered, any time you have a system which subscribes to ComponentHandleState and reads from a different component, you're in danger of getting stale data. I did a quick scan over all the subscriptions in content and most look pretty benign, though it's gonna be a bunch of work to untangle, especially with systems which raise further events in that subscription. SharedSolutionContainerSystem looks suspicious, for example, while FixtureSystem (in engine) seems to have a much clearer issue if the updates arrive out of order.

Component states should be authoritative and ordering should be irrelevant; anything that is co-dependent on another component should be handling that in its own state update by updating the UI.

I think the only exception to this is co-dependent compstates but afaik that only affects joints.

[metalgearsloth]

Also we need a content issue specifying technologydatabasecomponent should actually be handling the UI update in its own state handler.

[eoineoineoin]

Component states should be authoritative and ordering should be irrelevant; anything that is co-dependent on another component should be handling that in its own state update by updating the UI.

Yeah, I'm just concerned that there's a lot of hidden co-dependent states in there. The SharedSolutionSystem for example, subscribes to ComponentHandleState for SolutionComponent, but in that callback, reads a ContainedSolutionComponent, writes to a SolutionManagerComponent, and also raises a SolutionChangedEvent which is then handled by a zillion other systems, reading who-knows-how-many other components.

Also we need a content issue specifying technologydatabasecomponent should actually be handling the UI update in its own state handler.

Can look into this; what would be your ideal solution here? Would the TechnologyDatabaseComponent handler reach into the SharedUserInterfaceSystem? Or raise a generic "Update UI" event? Remove any component reads from the UI code by packing data into a BoundUserInterfaceState update? Something else?

Whatever we do to content here is (hopefully) obsoleted by this PR, but there might need to be a huge amount of changes for this; there's two patterns used in UI code- one is that an instance of a BoundUserInterface, which will read some other component(s) on the entity to populate the UI. The alternative (fairly common) pattern is to pack all the data necessary for the UI into the BoundUserInterfaceState which is sent from the server. Any instances of the former pattern are liable to be hit by the bug fixed in this PR.