Archipelago icon indicating copy to clipboard operation
Archipelago copied to clipboard

Core: Split CollectionState into per player PlayerStates

Open alwaysintreble opened this issue 1 year ago • 2 comments

What is this fixing or adding?

Splits up CollectionState into individual PlayerStates for each player. Also allows for a fresh, or all state player state to be created for pre_filling, with less data overhead. I'm not fully happy with the result as it currently is, but it's backwards compatible (other than stale) and should give us a small step towards something better in the future. Originally attempted to add functionality such that a PlayerState object could be created and used directly for a fill step, but it was incredibly messy. Should maybe be revisited in the future, when more code can "expect" that sort of behavior.

How was this tested?

Lots of breaking stuff and debugging. Changed WorldTestBase to use a per player PlayerState.

If this makes graphical changes, please attach screenshots.

alwaysintreble avatar Nov 05 '23 07:11 alwaysintreble

I had a random thought about this today, from a cleanliness perspective I wonder if it would be desirable to additionally move (or replicate) logicmixin functionality onto player state. My understanding is that generally it would be operating on single worlds anyway so sandboxing that would be nice. Probably out of scope of this PR though?

BadMagic100 avatar Apr 29 '24 23:04 BadMagic100

I think it's a good idea to further split up state even more than I've done here, to the point where CollectionState is essentially just a container for the per player states, and having all of the actual functionality on per player states. It's out of scope to go much further with this PR though. LogicMixin would probably need to be modified such that the methods are attributed to that particular world, and only added to PlayerStates that are created for players of that game.

alwaysintreble avatar May 07 '24 15:05 alwaysintreble