Page MenuHomeWildfire Games

Use victory conditions json's in Atlas
ClosedPublic

Authored by Stan on Oct 27 2019, 4:11 PM.

Details

Reviewers
Silier
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP23881: Fix Atlas crash introduced by rP23847
rP23847: Use victory conditions json's in Atlas
Trac Tickets
#5068
Summary

See #5068

TODO:
Help me fix the warning
2>source\third_party\jsonspirit\json_spirit_value.h(586): warning C4505: 'json_spirit::value_type_to_string': unreferenced local function has been removed (compiling source file ..\..\..\source\tools\atlas\AtlasUI\ScenarioEditor\Sections\Map\Map.cpp)

Test Plan

Open atlas

  • Try saving a map and loading it and make sure the checkboxes of the gamemodes are chacked
  • Make sure they follow the previous naming conventions
  • Make sure it works for all mods
  • Add a new gamemode, make sure it appears in atlas without needing to recompile.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Stan created this revision.Oct 27 2019, 4:11 PM
Stan updated the Trac tickets for this revision.

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/489/display/redirect

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

Linter detected issues:
Executing section Source...

source/simulation2/Simulation2.h
|   1| /*·Copyright·(C)·2018·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2018"

source/simulation2/Simulation2.h
|  47| class·CSimulation2
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCSimulation2{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
|  25| #include·<vector>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceAtlasMessage{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
| 211| »   »   );
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.

source/tools/atlas/GameInterface/Handlers/PlayerHandlers.cpp
|   1| /*·Copyright·(C)·2011·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2011"

source/tools/atlas/GameInterface/Handlers/PlayerHandlers.cpp
| 211| The line belonging to the following result cannot be printed because it refers to a line that doesn't seem to exist in the given file.
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
| 211| »   CMapWriter·writer;
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

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

Silier requested changes to this revision.Dec 1 2019, 11:52 AM

It works nice, but there is problem aside small styling.

Conquest inserts both structures and units victory scripts and that is reason why there have been OnConquestChanged.

What you could do to solve that more generally is to parse victory data and create some sort of map where would be condition as a key and list of other conditions that needs to be disabled as value.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
367 ↗(On Diff #10210)

pls new line above

371 ↗(On Diff #10210)

keep new empty line

source/tools/atlas/GameInterface/Handlers/PlayerHandlers.cpp
42 ↗(On Diff #10210)

dont remove this

source/tools/atlas/GameInterface/Messages.h
230 ↗(On Diff #10210)

spaces like above

This revision now requires changes to proceed.Dec 1 2019, 11:52 AM
Stan updated this revision to Diff 10460.Dec 2 2019, 12:22 PM
Stan marked 4 inline comments as done.

Fix styling issues, and read the data in the json file to know what to disable enable depending on what's on.

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/671/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

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

Silier requested changes to this revision.Dec 6 2019, 7:23 PM

victory conditions are not disabled correctly when done by code.

Steps to reproduce:
Enable conquest units
Enable conquest structures
Enable conquest

Save the map

In map xml file there are all 3 conditions present:

"conquest",
"conquest_structures",
"conquest_units"
This revision now requires changes to proceed.Dec 6 2019, 7:23 PM
Stan updated this revision to Diff 10506.Dec 7 2019, 12:40 PM
Stan edited the summary of this revision. (Show Details)

Move detection before saving settings to avoid victory conditions being incorrect.

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/702/display/redirect

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

Linter detected issues:
Executing section Source...

source/tools/atlas/AtlasObject/JSONSpiritInclude.h
|   1| /*·Copyright·(C)·2015·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2015"

source/simulation2/Simulation2.h
|  45| class·CSimulation2
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCSimulation2{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
|  25| #include·<vector>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceAtlasMessage{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
| 211| »   »   );
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
| 211| »   CMapWriter·writer;
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

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

Stan updated this revision to Diff 10514.Dec 7 2019, 5:13 PM

Fix bug when loading maps in a row. Fix reloading always ticking conquest.

Vulcan added a comment.Dec 7 2019, 5:16 PM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/706/display/redirect

Vulcan added a comment.Dec 7 2019, 5:30 PM

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

Linter detected issues:
Executing section Source...

source/tools/atlas/AtlasObject/JSONSpiritInclude.h
|   1| /*·Copyright·(C)·2015·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2015"

source/simulation2/Simulation2.h
|  45| class·CSimulation2
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCSimulation2{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
|  25| #include·<vector>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceAtlasMessage{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
| 211| »   »   );
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
| 211| »   CMapWriter·writer;
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

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

Silier added inline comments.Dec 7 2019, 5:41 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
251 ↗(On Diff #10514)

is this Enable really needed with L237?

274 ↗(On Diff #10514)

move that inside the block above

298 ↗(On Diff #10514)

what about to add "code" entry to victory conditions so we do not need to do this?
also title does not necessarily need to follow file name

Stan updated this revision to Diff 10516.Dec 7 2019, 5:48 PM
Stan marked 3 inline comments as done.

Fix some inlines

Vulcan added a comment.Dec 7 2019, 5:51 PM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/708/display/redirect

Vulcan added a comment.Dec 7 2019, 6:06 PM

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

Linter detected issues:
Executing section Source...

source/tools/atlas/AtlasObject/JSONSpiritInclude.h
|   1| /*·Copyright·(C)·2015·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2015"

source/simulation2/Simulation2.h
|  45| class·CSimulation2
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCSimulation2{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
|  25| #include·<vector>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceAtlasMessage{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
| 211| »   »   );
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
| 211| »   CMapWriter·writer;
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

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

Stan added inline comments.Dec 8 2019, 2:19 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
251 ↗(On Diff #10514)

Only if input is bogus, but no, I'll remove it.

298 ↗(On Diff #10514)

Would be only used in Atlas, doesn't that suck?

Silier added a subscriber: Itms.Jul 16 2020, 2:02 PM

Some comments.
I ll retest this.
Also would need rebase (years)

source/tools/atlas/AtlasObject/JSONSpiritInclude.h
1 ↗(On Diff #10516)

^

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
242 ↗(On Diff #10516)

this might be inlined

361 ↗(On Diff #10516)

without brackets

362 ↗(On Diff #10516)

no need of ; ref @Itms

298 ↗(On Diff #10514)

hmm, after discussion months ago and long thinking, I can live with parsing file names as we rely on file names matching conditions anyway for js gamesetup part.

After rebase I can accept.

Stan updated this revision to Diff 12767.Jul 18 2020, 6:38 PM
Stan marked 5 inline comments as done.

Inlines, copyright

Stan updated this revision to Diff 12768.Jul 18 2020, 6:48 PM

Update copyright again

Silier accepted this revision.Jul 18 2020, 6:51 PM

Functionality works.
Did not discover broken victory conditions in map files when saving them.
This is good for removing hardcoding.
Thank you for patch.
(for more info, see comment history)

This revision is now accepted and ready to land.Jul 18 2020, 6:51 PM

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

Linter detected issues:
Executing section Source...

source/simulation2/Simulation2.h
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/simulation2/Simulation2.h
|  45| class·CSimulation2
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCSimulation2{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
|  25| #include·<vector>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceAtlasMessage{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
| 211| »   »   );
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.

source/simulation2/Simulation2.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
| 211| MESSAGEHANDLER(SaveMap)
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

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

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

Linter detected issues:
Executing section Source...

source/simulation2/Simulation2.h
|  45| class·CSimulation2
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCSimulation2{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
|  25| #include·<vector>
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceAtlasMessage{' is invalid C code. Use --std or --language to configure the language.

source/tools/atlas/GameInterface/Messages.h
| 211| »   »   );
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.

source/tools/atlas/GameInterface/Handlers/MapHandlers.cpp
| 211| MESSAGEHANDLER(SaveMap)
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

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

This revision was automatically updated to reflect the committed changes.
Owners added subscribers: Restricted Owners Package, Restricted Owners Package, Restricted Owners Package.Jul 18 2020, 7:41 PM