Build has FAILED

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

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

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

Code complexity aside, sure a whitelist isn't preferable?
(as in people forgetting to change the un/placeable file rather having some valuable templates not placeable instead of having some templates that shouldn't ever be placed available in atlas?)

In D878#34308, @elexis wrote:

Code complexity aside, sure a whitelist isn't preferable?

We have one now, that limits modders to using the exact same folders as we do, or finding out about that filter file. And templates that shouldn't be placed do tend to be named "template_*" or put in one of the folders with unplaceable templates.

(as in people forgetting to change the un/placeable file rather having some valuable templates not placeable instead of having some templates that shouldn't ever be placed available in atlas?)

Something non-placeable will not make any sense to the player, so if they still try to place those they get what they deserve (after having added a few of those themselves, else they wouldn't show up).
Having new templates in some folder (e.g. units/romans/) not show up because they weren't added to the placeables filter is a lot worse IMO.

(unplaceables ending up in atlas might cause atlas errors or invisible items which players might not be able to delete and people not knowing how to delete it in the XML file.
If some new units don't show up in atlas, that might go unnoticed indeed (as opposed to placeables that shouldnt be placeable). So approach seems correct.)

In D878#34375, @elexis wrote:

(unplaceables ending up in atlas might cause atlas errors or invisible items which players might not be able to delete and people not knowing how to delete it in the XML file.

I still consider adding new folders with unplaceable templates to be the uncommon case, and that is more likely to be done by some programmer and a relatively quick search should point out that there is another file to update.

If some new units don't show up in atlas, that might go unnoticed indeed (as opposed to placeables that shouldnt be placeable). So approach seems correct.)

Any complaints about the pointer to a pair of pointers or the TODOs in the code apart from that?

I suspect that we might be able to remove FindTemplates since there are nearly no users of that (and I'm not really sure if the AI needs to know about non-placeable templates that are not special_filter ones (which it also doesn't need to know, but just query for them). And that also filters for template_ (though it does hard-code that, which might be faster.

@leper: can you 'rebase' the patch? (the last hunk doesn't apply right now)

In D878#36094, @fatherbushido wrote:

@leper: can you 'rebase' the patch? (the last hunk doesn't apply right now)

I guess that might be caused by me working on that thing on top of my other changes (which in this case includes D863).

Also given D925 and the now given possibility of using subfolders I have been thinking of moving all those other unplaceable templates to within special/ or something, and then hard-coding that check (as is currently done for template_ in AddToTemplates and using that for everything again. (If anything needs to know about other things we should do that for those things specifically.)

Also given D925 and the now given possibility of using subfolders I have been thinking of moving all those other unplaceable templates to within special/ or something, and then hard-coding that check (as is currently done for template_ in AddToTemplates and using that for everything again. (If anything needs to know about other things we should do that for those things specifically.)

It looks that last plan can do the job.