Page MenuHomeWildfire Games

[PetraAI] - Only try to subtract resource gift from "waitingfortribute" if gift contains required resource.
ClosedPublic

Authored by Freagarach on Apr 18 2020, 9:10 PM.

Details

Summary

Before rP23065 a tribute would send all resources in the object though all but one with an amount of "0".
After the aforementioned commit changed that to only send the resource that was sent. However, PetraAI still expected a value for all resources, as reported by @elexis.

Test Plan

Verify that with the replay attached to the ticket will give the mentioned warning, but the warning is not shown with this patch.
Check for more locations in PetraAI where this same assumption is made.

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

Freagarach created this revision.Apr 18 2020, 9:10 PM
Owners added a subscriber: Restricted Owners Package.Apr 18 2020, 9:10 PM

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

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

Silier accepted this revision.Apr 19 2020, 1:59 PM

Event not used elsewhere in petra nor common-api.
Petra becomes ally after tribute is sent.
Petra sends reminder when tribute is not sent whole yet.
No warning shown when another resource is tributed.
Thank you for the fix.

This revision is now accepted and ready to land.Apr 19 2020, 1:59 PM
Freagarach edited the test plan for this revision. (Show Details)Apr 22 2020, 9:06 AM

(On style, x in y seems nicer than y[x] !== undefined, but if one has to test b in a && c in a[b] && d in c[a[b]] it can become easier to read if one writes it as a[b] !== undefined && c[a[b]] !== undefined && d[c[a[b]]] !== undefined since its in hierarchical order, so I always avoided the x in y statement. Also it has the advantage that the term evt.amounts[request.type] would appear twice, making it easier for the reader to notice, whereas it needs some more milliseconds of human evaluation to notice that the in-expression tests for the same as below. Not that it matters how you write it, especially in this diff.)

If Angen confirmed that this is correct and sufficient I would trust him with the review based on his track record so far.
Thanks for finding this issue and writing the patch you two.

Does it work too if one requested multiple resources? (Should it?)

it does not.
Petra requests only one resource and waits for that one.

Does it work too if one requested multiple resources? (Should it?)

It didn't work before this and will not work hereafter. (Yes, eventually it should, I guess.)

Right, the other way (sending Petra an offer to accept alliance if X wood and Y food are sent) is not implemented. And

No warning shown when another resource is tributed.

Then it ought to be correct and complete within the predefined scope.

Thanks both for the look and thanks @Angen for the commit :)