Something wrong with TreasureCollected event in treasure islands scenario map

[fatherbushido]

In a21 or current svn r19287, when collecting a treasure on the treasure islands scenario map (where the goal is to collect all treasures) the TreasureCollected method is called many times and so it leads to the victory. That event is called by ResourceGatherer.TryInstantGather (That's the map given in example in the wiki doc ​http://trac.wildfiregames.com/wiki/Triggers#Examples) See also what happens with the AI

[elexis]

Got replay of insta victory? In my test I had to collect about 4 treasures

[fatherbushido]

4 would be false too. You tried to gather the first one then stop (don't auto gather)?

[elexis]

yep, revealed map, clicked that one, then with a merchantship the closest treasure around

[fatherbushido]

a21 replay

[fatherbushido]

in fact there is different kind of treasure (use a cargo treasure to reproduce) The dashed remark makes no sense. I succeed to reproduce with the persian metal treasure on the starting island.

[fatherbushido]

(unrelated but there are some treasures templates in gaia/ and some in other/, there should be some logic but I didn't really find it)

See also around L2400 of UnitAI (In fact, we don't leave and loop on trying to gather the treasure - dunno if it's related to the I am too busy cheat-code)

[elexis]

This doesnt seem related to the map but UnitAI or so.

If you add an warn("Pickup:" + this.entity); to the if (cmpResourceGatherer.TryInstantGather(this.gatheringTarget)) line in UnitAI which initiates the TreasureCollected event, you see that that it is somehow called for many entities, despite the fact that we sent the order only to one.

[elexis]

Argh, I didn't see your edited comment again xD

[fatherbushido]

(See also ResourceSupply.TakeResources) In fact with the i am too busy code, the destroy message (for the treasure) is sent but we try again to gather him and that many times (so we don't add the ress, but we try to gather it and call the trigger event). So happy that this map revealed that bug. I wonder if we can't have that with some stats without the cheat code. Changing the treasure rate gathering in unit template allow to reproduce the bug without the time cheat code. I see several fix but I don't want to just add an hack.

[fatherbushido]

Now (r20140) after taking the first treasure, I still have the bug but -that's the purpose of the update- also an error:

ERROR: JavaScript error: gui/session/messages.js line 899
TypeError: Engine.GetGUIObjectByName(...) is undefined
addChatMessage@gui/session/messages.js:899:51
playersFinished@gui/session/session.js:549:1
g_NotificationsTypes.defeat@gui/session/messages.js:316:3
handleNotifications@gui/session/messages.js:531:4
onSimulationUpdate@gui/session/session.js:824:2
__ eventhandler263 (simulationupdate)@sn simulationupdate:0:1__

(introduced by r19955)

[fatherbushido]

Also the treasure in the middle island seems unpickable (a fix like r19553 could be done)

[elexis]

In 20156:

Add two missing checks in the EndgameManager for players who already had won or were defeated in rP19955 as reported by fatherbushido, refs #4382.

Add a warning if a Trigger script (like the one on Treasure Islands, refs #4507) tries to marks a player as won who had already won or lost.
Merge a duplicate call in the Player component.

[fatherbushido]

Simple fix. Needs to figure if that approach is better than an owner check in the calling code.

[fatherbushido]

In 20193:

Tag exhausted resource as not available in ResourceSupply component. Fix #4507. Reviewed by leper.
Differential Revision: ​https://code.wildfiregames.com/D902