Build is green

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2186/ for more details.

To do: fix maps/scripts/CaptureTheRelic.js lines 4 and 5:

let catafalqueTemplates = shuffleArray(cmpTemplateManager.FindAllTemplates(false).filter(
  name => name.startsWith("other/catafalque/")));

Is “contains” instead of “startsWith” allowed?

Check maps/scripts/CaptureTheRelic.js. Sometimes only grepping for a part of the full name is helpful.

In D994#38837, @leper wrote:

Check maps/scripts/CaptureTheRelic.js. Sometimes only grepping for a part of the full name is helpful.

Actually I did before uploading this patch, however I did not know exactly how to fix it, hence my previous post.
Can startsWith contain an asterisk * abbreviation? E.g.:

Is “contains” instead of “startsWith” allowed?

(That's what we have. In recent javascript, it's includes.)

In D994#38840, @fatherbushido wrote:

Is “contains” instead of “startsWith” allowed?

(That's what we have. In recent javascript, it's includes.)

Eh? Should I replace line 4 with:
name => name.contains("_catafalque")));
or with:
name => name.includes("_catafalque")));
or with something else?
Als note that the above applies to the generic template (template_unit_catafalque) as well; is that a problem?

In D994#38841, @Nescio wrote:

In D994#38840, @fatherbushido wrote:

Is “contains” instead of “startsWith” allowed?

Our engine uses SpiderMonkey 38 currently, (https://trac.wildfiregames.com/wiki/SpiderMonkey) that will be switched to 45 hopefully soon, so that we are in sync with Mozilla developments the first time since the development of this game began afaik.
This means that we can currently only use ES6 functions.
https://en.wikipedia.org/wiki/ECMAScript#6th_Edition_-_ECMAScript_2015

But includes is ES7
http://www.ecma-international.org/ecma-262/7.0/#sec-array.prototype.includes

There is endsWith however if that can be used here.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/endsWith

In D994#38876, @elexis wrote:

In D994#38841, @Nescio wrote:

In D994#38840, @fatherbushido wrote:

Is “contains” instead of “startsWith” allowed?

There is endsWith however if that can be used here.

Thanks! However, does endsWith excludes the extension or has .xml to be included then? And the generic template is not a problem?

Our engine uses SpiderMonkey 38 currently, (https://trac.wildfiregames.com/wiki/SpiderMonkey) that will be switched to 45 hopefully soon,

I wouldn't hold my breath, and I haven't seen anyone working on that for quite a while.

so that we are in sync with Mozilla developments the first time since the development of this game began afaik.

Nope, we had that with 31 (which was released not very long after we switched to 24), and 38. And since ESR52 has also been out for a while, the only reason why we'd be at the latest version is that upstream is slow with releasing standalone packages. I guess one could just use the ESR tarballs, but meh.

Hm, I liked that catafalque subdirectory, but fatherbushido is right that its more conform if it's in the same directory as all the other units.

In D994#38836, @Nescio wrote:

To do: fix maps/scripts/CaptureTheRelic.js lines 4 and 5:

let catafalqueTemplates = shuffleArray(cmpTemplateManager.FindAllTemplates(false).filter(
  name => name.startsWith("other/catafalque/")));

Is “contains” instead of “startsWith” allowed?

All the templatesare cached and this part is only executed after the first turn, so we can do a better check than testing for the filename (as in testing for the template property, Relic class or something)

In D994#38921, @elexis wrote:

All the templatesare cached and this part is only executed after the first turn, so we can do a better check than testing for the filename (as in testing for the template property, Relic class or something)

“Relic” class would work; how to translate that into code? (My experience with js programming is non-existent.)

In D994#38922, @Nescio wrote:

In D994#38921, @elexis wrote:

“Relic” class would work; how to translate that into code? (My experience with js programming is non-existent.)

Code knowledge can be substituted with curiosity and adaptation.
Trigger script code is simulation code, so you can do a keyword search in simulation/components/, for instance grep -Ri class.
Though the simulation mostly checks for classes of existing entities, not classes of templates.
Take a look at Regicide.js line 26 (you can probably take that code and put it into the Relic filter function`).

In D994#38937, @elexis wrote:

In D994#38922, @Nescio wrote:

In D994#38921, @elexis wrote:

“Relic” class would work; how to translate that into code? (My experience with js programming is non-existent.)

Code knowledge can be substituted with curiosity and adaptation.

In principle I agree. However, it does help if one has an idea of what one is doing. E.g. I was informed yesterday of sed and I subsequently tried it out creating this patch, because sed was easy enough for me to understand.

In D994#38937, @elexis wrote:

Trigger script code is simulation code, so you can do a keyword search in simulation/components/, for instance grep -Ri class.
Though the simulation mostly checks for classes of existing entities, not classes of templates.

Although I see there is a difference, I don't fully comprehend how it works.

In D994#38937, @elexis wrote:

Take a look at Regicide.js line 26 (you can probably take that code and put it into the Relic filter function`).

Thanks for the tip. I've looked at it, several times, and made several attempts, and although I can imagine it does something similar to what is wanted in CaptureTheRelic.js, I failed to figure out how it works, whether I need only line 26 or the whole function (18--41) or another selection, which parts I can copy and keep unchanged, which I have to edit, and which to omit, etc. In the end I was convinced I was wasting my time (and perhaps yours as well), so I decided to simply replace “startsWith” with “endsWith”.

If you attempted to find out how this worked but spent too much time, just ask again.

These are the relevant lines:

		let identity = cmpTemplateManager.GetTemplate(templateName).Identity;
		let classes = GetIdentityClasses(identity);
		if (classes.indexOf("Hero") == -1 ||

This can become:

	let catafalqueTemplates = shuffleArray(cmpTemplateManager.FindAllTemplates(false).filter(
			name => GetIdentityClasses(cmpTemplateManager.GetTemplate(name).Identity).indexOf("Relic") != -1

Build is green

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2193/ for more details.

In D994#39039, @elexis wrote:

These are the relevant lines:

		let identity = cmpTemplateManager.GetTemplate(templateName).Identity;
		let classes = GetIdentityClasses(identity);
		if (classes.indexOf("Hero") == -1 ||

This can become:

	let catafalqueTemplates = shuffleArray(cmpTemplateManager.FindAllTemplates(false).filter(
			name => GetIdentityClasses(cmpTemplateManager.GetTemplate(name).Identity).indexOf("Relic") != -1

Thanks!

In D994#39039, @elexis wrote:

If you attempted to find out how this worked but spent too much time, just ask again.

Are you sure you don't mind me asking? I don't like wasting time of other people.

Build is green

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2195/ for more details.

In D994#39046, @Nescio wrote:

In D994#39039, @elexis wrote:

just ask

sure?

That mostly depends on the amount of time requested, the amount of consideration given to the answer and finally the asked persons interest in the code area.
This patch for instance interests me more than the treasure one, because svn blames me for all the files changed here.

Diff looks correct and complete, so I guess I can commit this after finishing my patch if noone else comes prior.

In D994#39051, @elexis wrote:

svn blames me for all the files changed here.

Is it because I used git mv?
When I first started using phabricator, about two months ago, I noticed svn mv was not accepted by arc (bb confirmed it was broken, not just me); the trio mv, svn add, svn rm was more work and less pleasant to view. Recently I tried out a git clone instead and noticed that git mv is unproblematically accepted by arc and phabricator, at least at my end ...

(I never understand how I can preserve the file history by applying the patch. Neither svn patch with the downloaded patch nor arc patch seem to do that)

You could do the moves manually, then edit the diff (if needed), then apply that. (Or use a git-svn clone.)

@fatherbushido do you want to commit this or shall I?

I need to prepare a pizza so if you could do...

In D994#39067, @leper wrote:

(Or use a git-svn clone.)

bingo! A content tracker. (I need to import svn commits before the mirror is updated often anyway)

About git svn, I'm unable to import svn with git svn to my existing repository. Importing all commits fails all 1000 commits with too many objects that need to be pruned. Importing only the head revision fails with some tens of thousands objects that need to be pruned. Never finishes and I need to keep my branches. Perhaps I could do a second repository and set this a remote though.

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2199/ for more details.

To clarify, the selection group name can actually be anything, as long as it's unique, right? That it typically matches the template name is a sensible convention.

In D994#39139, @elexis wrote:

(No need to update the patch as I can't apply it anyway with the file moves, I just wait for a response or some time to pass as I just finished applying the patch manually)

Wait, you had to manually redo everything I did? That seems both annoying and ineffecient. Is that common practice with reviewing and implementing patches?

In D994#39127, @elexis wrote:

About git svn, I'm unable to import svn with git svn to my existing repository. Importing all commits fails all 1000 commits with too many objects that need to be pruned. Importing only the head revision fails with some tens of thousands objects that need to be pruned. Never finishes and I need to keep my branches. Perhaps I could do a second repository and set this a remote though.

By existing I presume cloned from the official git clone on github/gitlab. In that case you will have to do more work to get the same state as that repo, check one of the staff forum topics started by k776, I outlined the steps needed to get a repo that is compatible with that. Also check if you have access to the official clone, if not ask someone to give you that, then you can actually update that when needed (or get @Itms to allow you to trigger an update via a jenkins job or something). Then possibly add your current repo as a new remote, but then you will have to take extra care to not push your work to the official one. So better have one repo (or the job mentioned above) to update the repo, and your current repo to work on things.

(Or just do a non-public git-svn clone and work in that instead, for porting branches you might also want to look at git-format-patch.)

In D994#39146, @leper wrote:

Also check if you have access to the official clone, if not ask someone to give you that, then you can actually update that when needed (or get @Itms to allow you to trigger an update via a jenkins job or something).

I gave you access on GitHub @elexis, tell me if you also have a GitLab account and want the access there. For the Jenkins job, currently the repo updates happen on the code machine and I don't want to give Jenkins access to that. It's just easier to push to the repo in case of need IMO.

In D994#39175, @Itms wrote:

In D994#39146, @leper wrote:

I gave you access on GitHub @elexis

Thanks Itms

tell me if you also have a GitLab account and want the access there

Not yet, and, not yet.

In D994#39146, @leper wrote:

By existing I presume cloned from the official git clone

(Yes, git mirror clone. Then adding the git-svn branch to .git/config and just doing a git svn fetch or git svn fetch -r HEAD.)

then you can actually update that when needed

I guess after every svn commit would be a bit noisy with that one merge commit for each update. I'll figure out how to use this. (Due to Murphys Law, I definitely won't add a personal branch to something that is able to push to the official mirror.)

tell me if you also have a GitLab account and want the access there

Not yet, and, not yet.

Get one, the repos being out of sync would be bad (especially if someone does not pull from both before updating).