-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add Generate Map tab to the lobby map chooser. #21855
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
Conversation
3b4ef38
to
1ddf0f8
Compare
Rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got several crashes
OpenRA.FieldLoader+MissingFieldsException: LandTile, WaterTile
at OpenRA.FieldLoader.Load(Object self, MiniYaml my) in OpenRA/OpenRA.Game/FieldLoader.cs:line 591
at OpenRA.Mods.Common.Traits.ExperimentalMapGeneratorInfo.Parameters..ctor(Map map, MiniYaml my) in OpenRA/OpenRA.Mods.Common/Traits/World/ExperimentalMapGenerator.cs:line 234
at OpenRA.Mods.Common.Traits.ExperimentalMapGeneratorInfo.Generate(ModData modData, MapGenerationArgs args) in OpenRA/OpenRA.Mods.Common/Traits/World/ExperimentalMapGenerator.cs:line 515
at OpenRA.Mods.Common.Widgets.Logic.LobbyLogic.<>c__DisplayClass55_2.<.ctor>b__53() in OpenRA/OpenRA.Mods.Common/Widgets/Logic/Lobby/LobbyLogic.cs:line 286
OpenRA.FieldLoader+MissingFieldsException: LandTile, WaterTile
at OpenRA.FieldLoader.Load(Object self, MiniYaml my) in OpenRA/OpenRA.Game/FieldLoader.cs:line 591
at OpenRA.Mods.Common.Traits.ExperimentalMapGeneratorInfo.Parameters..ctor(Map map, MiniYaml my) in OpenRA/OpenRA.Mods.Common/Traits/World/ExperimentalMapGenerator.cs:line 234
at OpenRA.Mods.Common.Traits.ExperimentalMapGeneratorInfo.Generate(ModData modData, MapGenerationArgs args) in OpenRA/OpenRA.Mods.Common/Traits/World/ExperimentalMapGenerator.cs:line 515
at OpenRA.Mods.Common.Widgets.Logic.LobbyLogic.<>c__DisplayClass55_2.<.ctor>b__53() in OpenRA/OpenRA.Mods.Common/Widgets/Logic/Lobby/LobbyLogic.cs:line 286
Exception of type `System.NullReferenceException`: Object reference not set to an instance of an object.
at OpenRA.MiniYaml.FromString(String text, String name, Boolean discardCommentsAndWhitespace, HashSet`1 stringPool) in OpenRA/OpenRA.Game/MiniYaml.cs:line 398
at OpenRA.Network.UnitOrders.ProcessOrder(OrderManager orderManager, World world, Int32 clientId, Order order) in OpenRA/OpenRA.Game/Network/UnitOrders.cs:line 394
at OpenRA.Network.OrderManager.ReceiveImmediateOrders(Int32 clientId, OrderPacket orders) in OpenRA/OpenRA.Game/Network/OrderManager.cs:line 180
at OpenRA.Network.NetworkConnection.OpenRA.Network.IConnection.Receive(OrderManager orderManager) in /OpenRA/OpenRA.Game/Network/Connection.cs:line 350
at OpenRA.Sync.RunUnsynced[T](Boolean checkSyncHash, World world, Func`1 fn) in /OpenRA/OpenRA.Game/Sync.cs:line 192
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is still in proof of concept stage. Lots of polish still seems missing
This is
and is unrelated to this PR. |
the second crash happens when in skirmish you exit a lobby with a generated map and return. I recon we should not to hurry to merge this, but wait for a feature complete version and do full testing. There really isn't a need to break bleed. Some of the commits should be split off into separate PR's |
Agreed, but do feel it important for the core plumbing to be tested and sanity checked independently from the UI. I can split out some of the commits, but note that these won't then have any testcases. |
We can kee changes without a testcase here. Other's can be quickly reviewed |
e95362d
to
15bd4a1
Compare
Converted to draft, added the (still WIP) lobby UI, and split out a bunch of prereq PRs. |
948df0e
to
5de5e37
Compare
wouldn't having a round map actually increase symmetry? |
Yes, but you can't make a round map out of a rectangular area. |
9ab9998
to
5f4f184
Compare
Small partial update:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels much better. One more kink left; when you click change map in skirmish lobby, the map is still regenerated despite it being already generated
The issue is that, at this point, it hasn't been generated (in the form needed by the generator UI). The previous |
In that case we can handle that at a later point in time if it's necessary |
Another small partial update:
Remaining tasks:
|
91ca266
to
b233f46
Compare
UI updated, including a fix for previously generated maps. |
Generated maps are created at runtime using a mod-defined trait. They are embedded directly into replays and save games as they exist in memory only, and disappear after the game exits.
Updated with a fix for the save game crash. This happened if you try to load a save after closing and reopening the game. The generated map was not in the cache, so the server selected a different map to use instead... Moved the MapCache update logic earlier in the process to prevent this. Ready for re-review. |
Added a special case to avoid regenerating the map for skirmish/hosts, which significantly speeds up selecting a generated map. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you generate the map, and leave lobby, then when you return to lobby the map stays. However, when you click to change the map it regenerates.
I assume this bug happens because we don't remember map generation options when one exists skirmish lobby. Maybe it should be put into the lobby skirmish map options serialization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, additional fixes can be done in followups
This PR introduces a third classification of map (together with local and remote):
MapClassification.Generated
, which is constructed at runtime by passing an opaque yaml blob to a mod-defined map generation trait.The lobby random map generator UI is split into its own followup PR, so this includes a simple testcase that overrides the "Change Map" button to select a random map with default settings. Note that the default settings in RA fail for at least one of the tilesets (probably Interior?), so it may take a couple of button presses for it to work (and/or just test in TD).Depends on
#21849,#21850,#21859,#21860,#21861,#21868,#21869,#21870,#21871,#21876,#21877.Strongly recommend reviewing by commit.