Page MenuHomeWildfire Games

Queueing two return resource orders - bug fix
AbandonedPublic

Authored by der-spaete-jo on Sep 28 2018, 9:55 AM.

Details

Reviewers
elexis
temple
wraitii
Trac Tickets
#4132
Summary

If a unit is ordered to return resources and (before he finishes) gets another queued order to return resources at a different location, then the unit would return the resources at the first location and then bug its way to the second location (not showing the appropriate walking animation). This patch fixes this behavior. A unit which walks towards a resource deposit gets the state "INDIVIDUAL.RETURNRESOURCE.APPROACHING" if it "CanReturnResource"s and it gets the state "INDIVIDUAL.WALKING" if it cannot. The latter is the case, when the unit already dopped its resources, but still has a return resource order.

Test Plan

?

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

der-spaete-jo created this revision.Sep 28 2018, 9:55 AM
lyv added a subscriber: lyv.EditedSep 28 2018, 10:09 AM

Add yourself to gui/credits/programming.json
(I might have messed up that path)
Edit: It’s not mandatory and you can even add only your nick or initials. Just clearing that out.

Added the dif to programming.json to include myself in the credits.

bug fix and credits

lyv added a comment.Sep 28 2018, 12:18 PM

Is this the better solution though? One could argue that it’s better to get rid of the order completely.

One could argue that it’s better to get rid of the order completely.

To me it is quite reasonable, that the "return resource order" transforms into a simple "walk there order" if the unit cannot return resources. Also if the player actually wants to queue a walk order after the return resource order, the input sequence would be the same (right-click on a deposit and then shift+right-clicking on another deposit) and he would expect the second order to be processed.

lyv added a comment.Sep 28 2018, 7:07 PM

Not really a regression with your patch, but units would still show the same bugged behavior in the following situation.

  • click on a dropsite and then shift-click on another dropsite.
  • delete the first dropsite before the unit reaches it.
  • the unit would show the bugged behavior.

Putting it out here since it's also a variation of the same bug more or less.

Not really a regression with your patch, but units would still show the same bugged behavior in the following situation.

Ok I am able to reproduce this bug. I thought changing line 639 to

if (this.order.data.target && this.CanReturnResource(this.order.data.target, true))

is an easy fix, but it doesn't do the job.

I find it hard to debug this problem. I would like to log the order list and the state of a unit form time to time. However, you cannot call warn("...") during an event handler, because this just spams the console. My solution is to put the warn calls in UnitAI.prototype.Walk and if I want to know the values for a certain unit, I (queue-)order him to walk somewhere. Is there a better way to debug js scripts?

I suppose the event handler Order.ReturnResource is called more than once while the unit is approaching the dropsite. Before the dropsite is destroyed, the handler terminates in line 643, so the units state will be APPROACHING. After the dropsite is destroyed, the handler should throw an error/warining since this.order.data.target links to a destroyed building. So CanReturnResource cannot be called and the handler should end up in the else case, with the unit being just WALKING. Can someone clarify the logic and timing here?

Stan added a subscriber: Stan.Sep 29 2018, 3:18 PM

If the log is too spammy in the game remember you can still see the whole output in interestinglog.html. see wiki:GameDataPaths

lyv added a comment.Sep 29 2018, 5:34 PM

You can also select display selection details in the dev-overlay ingame. Alt-D would bring it up. It contains basic unitAI info such as unit state.

wraitii abandoned this revision.Jan 5 2019, 11:00 AM

Hey, guess what, rP22023 actually fixes a bug in svn.

So this was caused by us setting the next state ('INDIVIDUAL.RETURNRESOURCE.APPROACHING') when we already were in 'INDIVIDUAL.RETURNRESOURCE.APPROACHING', and we didn't re-enter, so we didn't `SelectAnimation('move')`.

rP22023 also fixed what smiley described above.

Regardless, thank you @der-spaete-jo for noticing this issue and the revision :)