Page MenuHomeWildfire Games

Fix Atlas crash introduced by rP23847
ClosedPublic

Authored by Stan on Jul 21 2020, 8:07 PM.

Details

Summary

rP23847 introduced a crash which happened each time someone used a non wxCheckbox control

Test Plan

Make sure everything works as expected, saving loading, editing fields

Remove conquest from binaries/data/mods/public/simulation/data/settings/victory_conditions/conquest_structures.json and binaries/data/mods/public/simulation/data/settings/victory_conditions/conquest_units.json and check that ticking conquest disable both of them. but that disabling it does nothing as it's handled by the rest of the code.

Event Timeline

Stan created this revision.Jul 21 2020, 8:07 PM
Stan added inline comments.Jul 21 2020, 8:08 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
330

There is something that doesn't work here for conquest, not sure what I do bad.

vladislavbelov added inline comments.Jul 21 2020, 8:16 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
125

Might pass id instead, since it has the long type.

330

What exactly doesn't work and how the break removing helps?

Stan added inline comments.Jul 21 2020, 8:17 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
125

True

330

Check conquest unit and structures

Uncheck one

Notice conquest is not disabled anymore

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2741/display/redirect

Silier added inline comments.Jul 21 2020, 8:30 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
330

Hmm. Found another issue in current svn state.
Both checked -> disabled.
Uncheck one -> ticked and enabled.

Uncheck conquest.
Check structure -> uncheck structure -> conquest is checked

vladislavbelov added inline comments.Jul 21 2020, 8:37 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
244

Don't use count for search, use find.

304

Brace on new line. Also too many code, might be:

for (char& ch : escapedTitle)
	if (ch == ' ')
		ch = '_';

Also it's a code duplication .

Stan updated this revision to Diff 12839.Jul 21 2020, 10:00 PM

Fix notes

Stan marked 5 inline comments as done and an inline comment as not done.Jul 21 2020, 10:00 PM
Stan added inline comments.
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
330

Fixed Angen issue by setting it to false always.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2743/display/redirect

Stan updated this revision to Diff 12841.Jul 21 2020, 10:08 PM

Fix name

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2744/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2746/display/redirect

Stan updated this revision to Diff 12844.Jul 21 2020, 11:31 PM

Fix the last bug

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2747/display/redirect

Stan updated this revision to Diff 12845.Jul 22 2020, 12:02 AM

Cleanup

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2748/display/redirect

Silier requested changes to this revision.Jul 22 2020, 9:14 PM
Silier added inline comments.
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
316

this is not working.
to test, remove "DisabledWhenChecked": ["conquest"], from one of the conquest specific files.

This revision now requires changes to proceed.Jul 22 2020, 9:14 PM
Stan updated this revision to Diff 12864.Jul 22 2020, 10:45 PM

Upload to the right diff T_T

Stan edited the test plan for this revision. (Show Details)Jul 22 2020, 10:53 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2759/display/redirect

vladislavbelov accepted this revision.Jul 24 2020, 4:56 PM

I've tested the patch on Windows 10, it works for me for any map settings editing. The patch looks ok, a bit wordy but ok for atlas.

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
121

Unnecessary empty line.

Silier accepted this revision.Jul 24 2020, 8:44 PM

both disabledwhenchecked and changeonchecked works correctly
cannot reproduce any crashes

This revision is now accepted and ready to land.Jul 24 2020, 8:44 PM
This revision was automatically updated to reflect the committed changes.
Stan marked 2 inline comments as done.
Owners added a subscriber: Restricted Owners Package.Jul 24 2020, 8:53 PM