[PATCH] Disable the idle workers button if there are no idle workers.

[svott]

minimap_panel.xml contains the following TODO

<!-- TODO: We should disable this button if there are no idle workers. -->

I think an implementation is quite useful.

The required image "minimap-idle-disabled.png" is already available

[svott]

I hope the implementation fits the engine requirements

(small think, the tooltip could change when no idle worker is available. but I don't know a simple solution for this small thing. Maybe someone knows a nice one-line solution)

EDIT: And I don't like the Engine.GuiInterfaceCall("FindIdleUnits", data) interface. Would be nicer to give a whole array with all classes instead using a loop and ask the Engine for each class separately.

[bb]

Some style issues:

• Use let instead of var whenever possible (especially in loops)
• Move the comment inside the if-statement outside (and probably rephrase it in "check for any idle workers")
• use ++i instead of i++
• availanle --> available?

---

Some more interesting stuff:

• When I have an idle worker and hover my mouse over the button, the highlighted button flashes and becomes the normal button again at next sim update --> updateIdleWorkerButton should set a highlighted button when mouse is on it.
• The updateIdleWorkerButton function can be called in the xml, instead of the updateIdleWorkerButton check and sprite setting (when previous is solved).

Replying to svott:

(small think, the tooltip could change when no idle worker is available. but I don't know a simple solution for this small thing. Maybe someone knows a nice one-line solution)

I think you can best make a new function in "tooltips.js" and call that from xml

[svott]

update session.js: removed the weaknesses mentioned in comment 2

[svott]

small changes of coding style in session.js

[elexis]

Looks almost good to go, but please update the following things:

• The code should not check for idle units everytime the button is hovered. The number of idle-units can only change in case of a simulation update (onSimulationUpdate). Hence disable the button in that method (maybe .enabled = false may or may not help) and if it's hovered, ignored the event. Another option would be to update a new global g_HasIdleWorker onSimulationUpdate to avoid duplicate checks.

• Feel free to add newlines to the XML or even move the code to a small init-helper function in session.js if you need more space.

hasIdleWorker:

• Inline data.
• ["Female", "Trader", "FishingBoat", "CitizenSoldier", "Healer"] shouldn't be defined twice. You can either make this a global g_WorkerTypes in session.js or add two helper methods (FindIdleWorkerUnit and FindIdleMilitaryUnit)
• The comment is not needed.

updateIdleWorkerButton:

• Don't assign newImage = ""; if you're going to overwrite it anyway some lines below, just leave it undefined.
• Better store Engine.GetGUIObjectByName("idleOverlay") than it's sprite, since you access that object twice.
• Nuke idleImage, idleDisabledImage, maybe even highlightedImage.
• Not sure if the code might look nicer if you'd also use a g_IsIdleWorkerButtonHighlighted.

[svott]

Thx for the hints.

I optimized the style stuff and introduced global vars to avoid multiple engine calls

(idleWorkerButton.enabled = false; does not help/do something)

[svott]

optimizations according to comment 3

[svott]

By the way, I know you guys are not the biggest fan of having too many small files. However, maybe it's nicer to transfer the idle worker logic into a separate file and keep session.js not too big/powerful.

[elexis]

Looks much better. Thanks for the patch.

• the comments in checkIdleWorkerState and updateIdleWorkerButton are not needed, since the code is simple enough
• l568 violates wiki:Coding_Conventions
• Since g_WorkerTypes is an array, you can also loop over it's items using for...of. Your implementation works too of course.
• Why did you change hasIdleWorker to checkIdleWorkerState? The former makes it easier to guess from the name what the function does imo.
• The GUIInterface function FindIdleUnits should be updated to directly check all classes, instead of calling the function for every class. But this is not necessarily in the scope of this ticket.
• updateIdleWorkerButton() still is a bit weird with that prefix treatment. The same code could be rewritten to:

  function updateIdleWorkerButton()
  {
  	g_HasIdleWorker = hasIdleWorker();
  
  	let idleWorkerButton = Engine.GetGUIObjectByName("idleOverlay");
  	let prefix = "stretched:session/";
  
  	if (!g_HasIdleWorker)
  		idleWorkerButton.sprite = prefix + "minimap-idle-disabled.png";
  	else if (idleWorkerButton.sprite != prefix + "minimap-idle-highlight.png")
  		idleWorkerButton.sprite = prefix + "minimap-idle.png";
  }
  

• The declaration of g_WorkerTypes should briefly mention that it is used for the idleworker-hotkey
• JSdoc comments should be used in the GUI
• Highlighting-bug If the button becomes enabled while the user hovers the button when it's disabled, the button will not show the highlighted image. To fix it, a new global g_IsIdleWorkerButtonHighlighted would have to be set on MouseEnter and MouseLeave. But who cares.
• Unmarked idlers for one turn: see #3748
• You are right about .enabled = false has no effect on the button. Might be a good practice to disable it if it's supposed to be disabled. But it's also a good practice not to have useless code.

[elexis]

In 17674:

Update the idle-worker-button onSimulationUpdate. Patch by svott, fixes #3736.

[elexis]

You might want to have a look at #2357. It has some nice features which should be included too. Showing the number of idle workers on the button is something that should be done to show the player the state.

[elexis]

In 17850:

Add svott (refs #3736) and fpre (refs #3205) to the contributors.

[elexis]

In 20601:

Fix idle worker button mess.

rP14819 mimicked the sprite_over button functionality which exists as a proper property since rP9.
rP17674 mimicked the sprite_disable and enabled button functionality.
The button now shows the proper highlight sprite if the button became enabled without one of the prior mouseover events being sent.
Removes duplicate hardcoded filenames and unneeded globals, functions and conditions.

Refs #2414, #3736
Differential Revision:
Patch By: temple