Paths

Table of Contentst

Return to making FindAllTemplates return all placeable templates again (switch to unplaceable filter).
ClosedPublic
Actions

Authored by • leper on Sep 29 2017, 5:35 AM.

Details

Reviewers

fatherbushido
elexis

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Owners Package	(Owns No Changed Paths)
Restricted Owners Package	(Owns No Changed Paths)

Commits

rP20246: Switch back to an unplaceable filter for templates.

Summary

This makes it easier for modders to use different template organization schemes.
(Other folders, more folders, different names, etc.)

This considers only templates starting with template_ or special/ to be unplaceable,
if you want more unplaceable templates start the name with template_ (in the root
template dir), or put them in special/ or a subfolder thereof.

This makes rubble/ placeable, as the only thing placing one of those does is
making them disappear very soon, so might not be quite what a user expected,
but that doesn't sound like a very good reason to prevent them from placing them.
Might be usable for a story map where the player starts in an area where things
were destroyed recently, but shouldn't stay around forever.

This also makes other/catafalque placeable. This previously was not placeable
due to a limitation of the related trigger script, and the underspecified interaction
with the number parameter in the gamesetup. See #4762.

Alternate proposal to D878.

Depends on D933 and D934.

Test Plan

Test if everything still works. Possibly disagree about making some things placeable. Also possibly complain about naming.

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

• leper created this revision.Sep 29 2017, 5:35 AM

Build has FAILED

Link to build: http://jenkins-master:8080/job/phabricator/2078/
See console output for more information: http://jenkins-master:8080/job/phabricator/2078/console

Harbormaster failed remote builds in B3275: Diff 3790!Sep 29 2017, 7:01 AM

Executing section Default...
Executing section Source...

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|  33| #include·"graphics/Terrain.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: ''.

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|  33| #include·"graphics/Terrain.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|  33| #include·"graphics/Terrain.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/554/ for more details.

fatherbushido requested changes to this revision.Sep 29 2017, 8:13 AM

This comment was removed by fatherbushido.

This revision now requires changes to proceed.Sep 29 2017, 8:13 AM

I did a mistake, that one doesn't have "request changes"

Edit: I finally find something. Don't forget to update binaries/data/mods/public/maps/scenarios/Units_demo.js

fatherbushido added inline comments.Sep 29 2017, 8:56 AM

source/simulation2/components/CCmpTemplateManager.cpp
219 ↗	(On Diff #3790)	We can remove it?

This makes rubble/ placeable, [...]

Fine with that.

Alternate proposal to D878.

That one is really simple and the hardcoded path doesn't really hurt.

Would also be great to have persistent rubble (perhaps with an undecayable special template?), but OT.

Was wondering wheather there might be some trigger script that want to loop over all placeable entities. Perhaps some demo map?
However if there is need for that, adding a boolean argument to FindTemplates is easier to maintain.
(Ah yes, Units_demo.js as fatherbushido said. Also temple created a variant for buildings for that once)

source/simulation2/components/CCmpTemplateManager.cpp
219 ↗	(On Diff #3790)	Must, since it doesn't compile with that reference

In D935#36573, @elexis wrote:

Would also be great to have persistent rubble (perhaps with an undecayable special template?), but OT.

I suspect doing it the other way around might be nicer. Having rubble be non-decaying and making the decaying one (that is currently placed by SpawnEntityOnDeath) use decay|template. or possibly decay_30s|template. Depending on how different all those are.

Was wondering wheather there might be some trigger script that want to loop over all placeable entities. Perhaps some demo map?
However if there is need for that, adding a boolean argument to FindTemplates is easier to maintain.
(Ah yes, Units_demo.js as fatherbushido said. Also temple created a variant for buildings for that once)

Yep, forgot about that one. Also these functions do take a path parameter so one could even loop over specific templates (I'm not sure if that path parameter is fully exposed to scripts, or if some point just passes ""), which could make it easy to place specific templates eg according to civ, or type (random mercenaries, etc). Or specific trees for a biome, by just placing those trees in a specific folder and looping over that for random maps.

I guess the general consensus is that hardcoding a second prefix for unplaceable templates is nicer than the other approach, unless someone didn't complain yet.

source/simulation2/components/CCmpTemplateManager.cpp
219 ↗	(On Diff #3790)	Guess I did forget something.

Fix the units demo map, actually remove a function.

Build has FAILED

Link to build: http://jenkins-master:8080/job/phabricator/2082/
See console output for more information: http://jenkins-master:8080/job/phabricator/2082/console

Harbormaster failed remote builds in B3282: Diff 3796!Sep 30 2017, 10:35 AM

Executing section Default...
Executing section Source...

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|  33| #include·"graphics/Terrain.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: ''.

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|  33| #include·"graphics/Terrain.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|  33| #include·"graphics/Terrain.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/557/ for more details.

ok
(some hunk failed to apply with arc patch but I did it manually)
D938 is needed before (just for the units demo map) else we spam non local entity with a decay component (assertion failure).

(some hunk failed to apply with arc patch but I did it manually)

One of the const ScriptInterface& ones or that one TODO for a comment. But yes, given that at least one of those has been sitting in one queue for a while I'm not very motivated to rearrange commits all the time.