Page MenuHomeWildfire Games

Auto-Queue feature
ClosedPublic

Authored by azayrahmad on Apr 17 2021, 6:00 AM.

Details

Summary

New command button to loop through queue indefinitely while within resources & population limits.

Test Plan

Single unit-producing structure:

  • Select the structure -> Ensure the Auto-Queue button is off (blue icon, tooltip to turn on)
  • Train some units (single & batch) -> Ensure the queue is not looping
  • Turn on the Auto-Queue -> Ensure the Auto-Queue button is on (red icon. tooltip to turn off)
  • Train some units (single & batch) -> Ensure the queue is looping as long as resource and pop available
  • Turn off the button again and train some units-> Ensure the queue is no longer looping

Multiple unit-producing structures:

  • Select multiple structures, turn on Auto-Queue, train units -> All selected structures are now on Auto-Queue
  • Select one of the structures, turn off Auto-Queue, train units -> The structure is no longer on Auto-Queue
  • Select all structures again -> both turn on and off Auto-Queue buttons are displayed.
  • Click the turn off Auto-Queue button -> All structures are now no longer on Auto-Queue

Non unit-producing structures:

  • Select the structure -> Auto-Queue button is not displayed

Hotkey test

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

azayrahmad requested review of this revision.Apr 17 2021, 6:00 AM
azayrahmad created this revision.

Hey, thanks for taking the time to create this patch.
You would need to add context to your patch, try this:

svn diff --diff-cmd diff -x "-U 99999" > changes.patch

wiki/SubmittingPatches seems to be not working correctly at the moment, try web.archive.org in the mean time.

I was able to apply your already uploaded patch, but I had problems with the .png files, they were not downloaded correctly. If I use the provided .png files from your GitHub repo, then the patch works for me.

PS: You could add screenshots to your description if you like

  1. Upload the files here: https://code.wildfiregames.com/file/upload/
  2. this generate a number, e.g. F1962183
  3. insert this number between some Braces ("curly braces")
  4. optional: increase the size with {..., width=400}

First of all, thanks for the patch! I really like this feature (as a casual player ^^ ).

Things I noticed:

  • When one entry could not be completed due to a resource shortage, autoqueue is stopped. Is that intended? It feels strange to me ^^ Especially since the GUI still says it is on ^^

Also, don't forget to add yourself to the programming credits.

binaries/data/mods/public/gui/session/unit_actions.js
1726 ↗(On Diff #17139)

We prefer structures, IIRC.

binaries/data/mods/public/simulation/components/GuiInterface.js
349 ↗(On Diff #17139)

This sounds like whether it is _currently_ producing units. I'd say you can use ret.production.entities.length directly in unit_actions.js.

349–350 ↗(On Diff #17139)

These should be part of the above object (ret.production.x).

binaries/data/mods/public/simulation/components/ProductionQueue.js
31 ↗(On Diff #17139)

What do you want to achieve with this template value? That autoqueue is on by default? You currently don't use it ;)

84 ↗(On Diff #17139)

Maybe something like: IsAutoQueueEnabled or IsAutoProducing. Now it sounds like the function returns the items that are automatically produced.

95 ↗(On Diff #17139)

No need for the return value. (Also below.)

103 ↗(On Diff #17139)

delete this.autoqueue;?

889 ↗(On Diff #17139)

We usually don't do this kind of ifs. But we check whether the conditions satisfy with an if and perform an action when it is.

if (condition)
  doX

It is a bit easier on the eye :)

binaries/data/mods/public/simulation/helpers/Commands.js
885 ↗(On Diff #17139)

let (And below.)

Freagarach added inline comments.Apr 17 2021, 8:11 AM
binaries/data/mods/public/gui/session/selection_panels_helpers.js
550 ↗(On Diff #17139)

Oh, and you need these helper functions only if you intend to add a hotkey to call these functions, else you can just inline this in unit_actions.

Make sure to triple check serialisation, as this introduces GUI - simulation interaction (though at a glance things look setup correctly)

(Can we get a screenshot of the GUI setup? I'd possibly like to make the icon optional (as in hidden behind an option) / make it relatively non-intrusive)

azayrahmad edited the summary of this revision. (Show Details)Apr 18 2021, 4:45 AM
azayrahmad edited the test plan for this revision. (Show Details)
azayrahmad updated this revision to Diff 17150.Apr 18 2021, 5:28 AM
azayrahmad marked 8 inline comments as done.

Updates addressing @Freagarach 's reviews.

I tried running command line as advised in wiki (svn diff --diff-cmd diff -x "-U 99999" > changes.patch), but found error on generating diff:

svn: E720002: Can't start process 'diff': The system cannot find the file specified.

So I simply run like this: svn diff -x "-U 99999" > changes.patch, but I'm afraid the images probably still not included.

azayrahmad marked 2 inline comments as done.Apr 18 2021, 5:40 AM

Things I noticed:

  • When one entry could not be completed due to a resource shortage, autoqueue is stopped. Is that intended? It feels strange to me ^^ Especially since the GUI still says it is on ^^

Yes. I just following 0AD current behavior, i.e. on pop limit: can add to queue but pending, on resource limit: cannot add to queue at all. It is similar to Rise of Nations' Infinite Queue.
So what happened in your case is the autoqueue is not stopped, it's merely removing units that require more resource than available, which in your case is the entire queue.

Freagarach set the repository for this revision to rP 0 A.D. Public Repository.Apr 18 2021, 7:24 AM
Freagarach added inline comments.Apr 18 2021, 10:59 AM
binaries/data/mods/public/gui/session/unit_actions.js
1735 ↗(On Diff #17150)

You don't use the entity IDs, so you don't need to map. Just call the plain function :)

binaries/data/mods/public/simulation/components/ProductionQueue.js
30–34 ↗(On Diff #17150)

I feel like this part belongs in a version 2: D2658.
What do you think?

wraitii requested changes to this revision.Apr 19 2021, 10:17 AM

I would like one change: add the items when production starts, so that there is always 2 items in the queue, the one being produced and the next one.
This is because otherwise Autoqueue is better than manual queuing, as it essentially performs "perfect" queuing, and doesn't incur a "working capital/in-flight" cost.

binaries/data/mods/public/gui/hotkeys/spec/ingame.json
163 ↗(On Diff #17150)

Activate auto-queue

binaries/data/mods/public/simulation/components/ProductionQueue.js
103 ↗(On Diff #17150)

I would prefer autoqueuing

889 ↗(On Diff #17150)

Probably want a GUI notification of some sort if it fails.

This revision now requires changes to proceed.Apr 19 2021, 10:17 AM
Freagarach added inline comments.Apr 19 2021, 10:22 AM
binaries/data/mods/public/simulation/components/ProductionQueue.js
889 ↗(On Diff #17150)

There is the infinite resources notification.

Freagarach added inline comments.Apr 19 2021, 10:26 AM
binaries/data/mods/public/simulation/components/ProductionQueue.js
889 ↗(On Diff #17150)

There is the infinite resources notification.

s/infinite/insufficient

wraitii added inline comments.Apr 19 2021, 10:27 AM
binaries/data/mods/public/simulation/components/ProductionQueue.js
889 ↗(On Diff #17150)

s/infinite/insufficient :p

But right never mind that's handled.

azayrahmad updated this revision to Diff 17211.Apr 24 2021, 4:40 AM
azayrahmad marked 8 inline comments as done.

New queue is added when production of current queue begins.
Change turn autoqueue on/off wording to activate/deactivate & autoqueuing.

nani added a subscriber: nani.Apr 24 2021, 6:13 AM

You might want to add some hotkeys for this.
I suggest these ones I already use that I think work well:
Alt+q to enable auto queue
Alt+w to disable auto queue

nani awarded a token.Apr 24 2021, 6:14 AM
Freagarach added a subscriber: Stan.EditedApr 26 2021, 8:55 AM

This seems to be good now. @wraitii can you confirm this is the behaviour you requested? I'm okay with it now, it means indeed a little more thought needs to go in before activating autoqueue.
@Stan can you check out the icons and give us your opinion about them?

@azayrahmad can you please set the repository correctly when updating the diff?

Freagarach set the repository for this revision to rP 0 A.D. Public Repository.Apr 26 2021, 8:56 AM
Stan added a comment.Apr 26 2021, 9:48 AM

I don't think they are great, but I suppose they could work as placeholders. If I read this patch correctly, the feature cannot be disabled, right? So we're introducing a significant gameplay change.
Is everything translated as it should?

Freagarach accepted this revision.May 2 2021, 12:21 PM

This is a nice feature.
There are some minor code-style issues, but I'll handle those when comittting.

@azayrahmad, can you upload a .zip with the images here?

This revision is now accepted and ready to land.May 2 2021, 12:23 PM

This is a nice feature.
There are some minor code-style issues, but I'll handle those when comittting.

@azayrahmad, can you upload a .zip with the images here?

Thank you very much, @Freagarach ! Here is the icon images that should be in art\textures\ui\session\icons. Thank you for your help.

Freagarach updated this revision to Diff 17342.May 3 2021, 8:16 AM
Freagarach edited the summary of this revision. (Show Details)
Freagarach edited the test plan for this revision. (Show Details)

Some slight adaptations, added a unit test.

Owners added a subscriber: Restricted Owners Package.May 3 2021, 8:16 AM
Freagarach added inline comments.May 3 2021, 8:20 AM
binaries/data/mods/public/simulation/components/ProductionQueue.js
87 ↗(On Diff #17342)

Changed this, because we don't need to read to the end of the function name to see what it does.

809–811 ↗(On Diff #17342)

To clarify the decision we made.

This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.May 5 2021, 8:12 AM

Congratz on landing your first patch, @azayrahmad!

nwtour added a subscriber: nwtour.May 6 2021, 4:24 PM

works, but does not look neat:
Long yellow text by default: "[Unassigned hotkey: session.queueinit.autoqueueon]" (expected "[ ]")
The icon on the right overshadows the gray border (the adjacent icons, on the contrary, are one pixel less than the black square)

Stan added a comment.May 6 2021, 4:27 PM

Yeah icons need rework.