Page MenuHomeWildfire Games

Store Garrison(Holder) in map file.
Needs ReviewPublic

Authored by Freagarach on Jan 23 2020, 9:52 PM.

Details

Reviewers
Angen
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#3008
Summary

Currently the garrison of an entity ought to be stored seperately in the mapfile as a script. That is doable.
However, D1958 allows entities to be created on init, thus when an entity is defined in a map it will create another one on map load (using the entity ID +1, creating havoc).
The only proper way of solving this would probably be to define that there is already an entity on the position.

This patch aims at fixing that the entities garrisoned in Atlas are stored and (re)garrisoned at init.


See also:


Possible extensions:

  • Make a UI to set garrisoned entities in Atlas.
  • Store more properties (such as turret position) in the mapfile.
Test Plan
  • Verify that a map with Garrison in a structure correctly saves that in the map-file.
  • Verify that loading a map with Garrison in an entity (such as Silica Nomad) correctly loads.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

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

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

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

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/229/display/redirect

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

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

Freagarach edited the summary of this revision. (Show Details)Jan 25 2020, 9:10 AM
Freagarach added inline comments.
source/graphics/MapReader.cpp
470–471

Not sure what to do with these? (And above.)

1004

This is obviously incorrect, but I do not know yet how to convert the XML information to the array.
Perhaps by splitting the text at every , and Garrison.insert(<element>.ToInt())?

source/graphics/MapWriter.cpp
351

How to convert the set to an array which can be saved in the tag?

elexis added a subscriber: elexis.Jan 26 2020, 3:49 PM

(and Sicilia Nomad showcase)

Freagarach edited the summary of this revision. (Show Details)Jan 26 2020, 4:12 PM

Seperate node per entity.

Freagarach planned changes to this revision.Thu, Jan 30, 10:05 PM

I still have to fix the build and do the showcase.

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

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

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

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/263/display/redirect

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

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

Freagarach updated this revision to Diff 11239.Sat, Feb 1, 11:03 AM
Freagarach retitled this revision from [WIP] - Store Garrison(Holder) in map file. to Store Garrison(Holder) in map file..
Freagarach edited the summary of this revision. (Show Details)
Freagarach added a reviewer: Restricted Owners Package.

Make it actually compile and work :)

Please note that the showcase still needs to be done, but it depends on D2376.

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

Linter detected issues:
Executing section Source...

source/simulation2/components/ICmpGarrisonHolder.h
|  23| class·ICmpGarrisonHolder·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonHolder:' is invalid C code. Use --std or --language to configure the language.

source/simulation2/components/ICmpGarrisonable.h
|  23| class·ICmpGarrisonable·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonable:' is invalid C code. Use --std or --language to configure the language.

source/graphics/MapReader.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/interfaces/Garrisonable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/interfaces/Garrisonable.js
|   1|    |-//Engine.RegisterInterface("Garrisonable");
|    |   1|+// Engine.RegisterInterface("Garrisonable");
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/interfaces/GarrisonHolder.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/interfaces/GarrisonHolder.js
|   1|    |-//Engine.RegisterInterface("GarrisonHolder");
|    |   1|+// Engine.RegisterInterface("GarrisonHolder");
|   2|   2| 
|   3|   3| /**
|   4|   4|  * Message of the form { "added": number[], "removed": number[] }
Executing section cli...

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

Freagarach updated this revision to Diff 11240.Sat, Feb 1, 11:37 AM

Updated Silica Nomad to use this feature.

Owners added a subscriber: Restricted Owners Package.Sat, Feb 1, 11:37 AM

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

Linter detected issues:
Executing section Source...

source/simulation2/components/ICmpGarrisonable.h
|  23| class·ICmpGarrisonable·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonable:' is invalid C code. Use --std or --language to configure the language.

source/simulation2/components/ICmpGarrisonHolder.h
|  23| class·ICmpGarrisonHolder·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonHolder:' is invalid C code. Use --std or --language to configure the language.

source/graphics/MapReader.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/interfaces/Garrisonable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/interfaces/Garrisonable.js
|   1|    |-//Engine.RegisterInterface("Garrisonable");
|    |   1|+// Engine.RegisterInterface("Garrisonable");
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/interfaces/GarrisonHolder.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/interfaces/GarrisonHolder.js
|   1|    |-//Engine.RegisterInterface("GarrisonHolder");
|    |   1|+// Engine.RegisterInterface("GarrisonHolder");
|   2|   2| 
|   3|   3| /**
|   4|   4|  * Message of the form { "added": number[], "removed": number[] }
Executing section cli...

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

Freagarach planned changes to this revision.EditedSun, Feb 2, 1:24 PM

Is it necessary to store the garrisonHolder also? It adds more room for errors and seems to add nothing really.
One could also add the garrisonHolder when adding the entities on game init.

source/graphics/MapReader.cpp
1

++

953

entity_id_t

Freagarach added a child revision: Restricted Differential Revision.Mon, Feb 3, 8:11 PM
Freagarach removed a child revision: D1958: Turrets and/or sub-units..
Freagarach updated this revision to Diff 11264.Mon, Feb 3, 8:14 PM
  • Bump year in MapReader.
  • Remove GarrisonHolder from map.
Stan added a subscriber: Stan.Mon, Feb 3, 8:34 PM
Stan added inline comments.
binaries/data/mods/public/maps/skirmishes/Sicilia_Nomad.xml
15599

I'm wondering if it shouldn't also be uid somehow? to make the link between entities.

source/graphics/MapReader.cpp
1006

I guess you could set the size of Garison to avoid resizing since you already know the definite size.

Can you use a range for loop?

Something along the lines of

for (const XMBElement& garr_ent : garrison) {
  XMBAttributeList attrs = garr_ent.GetAttributes();
  Garrison.insert(attrs.GetNamedItem(at_gid).ToInt());
}
source/graphics/MapWriter.cpp
350

Range loop here too.

source/simulation2/components/ICmpGarrisonHolder.cpp
38

range based for loop?

45

Range based for loop?

Vulcan added a comment.Mon, Feb 3, 8:47 PM

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

Linter detected issues:
Executing section Source...

source/simulation2/components/ICmpGarrisonHolder.h
|  23| class·ICmpGarrisonHolder·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonHolder:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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

Angen requested changes to this revision.Tue, Feb 4, 11:55 AM
Angen added a subscriber: Angen.
Relax-NG validity error : Extra element Entities in interleave
/public/maps/skirmishes/Sicilia_Nomad.xml:0: Relax-NG validity error : Element Scenario failed to validate content
This revision now requires changes to proceed.Tue, Feb 4, 11:55 AM

(To be fixed in ./binaries/data/mods/public/maps/scenario.rng ./binaries/data/mods/public/maps/scenario.rnc )

Freagarach marked 4 inline comments as done.Tue, Feb 4, 8:36 PM
Freagarach added inline comments.
source/graphics/MapReader.cpp
1006

I guess you could set the size of Garison to avoid resizing since you already know the definite size.

Hmm, I could not find a way to set the size actually?
(https://stackoverflow.com/questions/21930242/are-there-functions-to-request-capability-for-stdset)

Freagarach updated this revision to Diff 11270.Tue, Feb 4, 8:36 PM
  • Fix checkref warnings.
  • Use range for-loops.
Vulcan added a comment.Tue, Feb 4, 9:00 PM

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

Linter detected issues:
Executing section Source...

source/simulation2/components/ICmpGarrisonHolder.h
|  23| class·ICmpGarrisonHolder·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonHolder:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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

Angen added a comment.Wed, Feb 12, 9:18 PM

by this, code in /helpers/Setup.js using Garrison should be removed, as there should be only one way how to define the same data in one file.

source/simulation2/components/ICmpGarrisonHolder.cpp
34

does this need to require set ? if code could use vector, that copying could be avoided

46

you can remove these brackets :)

47

btw, this could pass array of them, no ? Instead one by one

Freagarach marked 2 inline comments as done.
  • Removed different manner of storing garrisoned entities.
  • Throw error when trying to garrison an entity in a entity that is not capable of holding entities.
  • Use std::vector in getEntities.
source/simulation2/components/ICmpGarrisonHolder.cpp
47

I tried that at first but it didn't work.

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

Linter detected issues:
Executing section Source...

source/simulation2/components/ICmpGarrisonHolder.h
|  23| class·ICmpGarrisonHolder·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonHolder:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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

Angen added a comment.Wed, Feb 19, 8:21 PM

btw, this could pass array of them, no ? Instead one by one

I tried that at first but it didn't work.

It works

virtual void SetInitEntities(std::vector<entity_id_t> entities)
{
   m_Script.CallVoid("AddInitGarrisonEntity", entities);
}

You just need to change type of Garrison from set to vector and edit AddInitGarrisonEntity to work with array.
And one bonus. When Garrison is vector, you can use resize to do what Stan wanted.

Freagarach planned changes to this revision.Thu, Feb 20, 7:54 PM
Freagarach updated this revision to Diff 11411.Fri, Feb 21, 7:37 PM
Freagarach marked 2 inline comments as done.

Use std::vector.

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

Linter detected issues:
Executing section Source...

source/simulation2/components/ICmpGarrisonHolder.h
|  23| class·ICmpGarrisonHolder·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonHolder:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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

Angen requested changes to this revision.Fri, Feb 21, 9:41 PM
Angen added inline comments.
binaries/data/mods/public/simulation/components/GarrisonHolder.js
698

The name says add but comment and code says set

708

this is changing postcondition of the function.
this.initGarrison was always undefined after this function was called, now it is not the case.

source/graphics/MapReader.cpp
1047

can be just if (Garrison.size())

This revision now requires changes to proceed.Fri, Feb 21, 9:41 PM
Stan added inline comments.Sat, Feb 22, 10:45 AM
source/graphics/MapReader.cpp
1053

Maybe it should state that it has no garrisonholder component. One could also display the player set above if available.

Freagarach updated this revision to Diff 11440.Thu, Feb 27, 6:08 PM
Freagarach marked 4 inline comments as done.
Freagarach edited the test plan for this revision. (Show Details)
  • SetInitGarrison.
  • if (Garrison.size()).

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

Linter detected issues:
Executing section Source...

source/simulation2/components/ICmpGarrisonHolder.h
|  23| class·ICmpGarrisonHolder·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonHolder:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

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