Opened 10 years ago

Last modified 5 years ago

#2357 new enhancement

[PATCH] Notify user when workers are out-of-work

Reported by: sanderd17 Owned by: Josh
Priority: Should Have Milestone: Backlog
Component: UI – In-game Keywords: patch
Cc: Patch:

Description

Idle workers can be found through the button right next to the minimap, but it would be nice if the button could show if there are idle units without clicking on it.

The blinking shouldn't be too obtrusive, as it will happen often in late game.

Attachments (20)

idle_worker_blink.patch (16.9 KB ) - added by Michael 10 years ago.
Effect is working but patch need some improvments…
idle_worker_blink_v2.patch (18.6 KB ) - added by Michael 10 years ago.
Made the code better. Thx to leper and sanderd17 for hints.
idle_worker_blink_v3.patch (18.8 KB ) - added by Michael 10 years ago.
Moved ownershipChanged to player; added label.
idle_worker_blink_v4.patch (19.2 KB ) - added by Michael 10 years ago.
Fixed problem with smooth reset. (opacity)
idle_worker_blink_v5.patch (19.1 KB ) - added by Michael 10 years ago.
Moved value to GetSimulationState() var.
player.xml (1.3 KB ) - added by Michael 10 years ago.
Updated player.xml but failed to change schema.
idle_worker_blink_v6.patch (22.1 KB ) - added by Michael 10 years ago.
Now works with class definition in player.xml file.
savegame-0020.0adsave (1.2 MB ) - added by Michael 10 years ago.
Save game for counter bug.
idle_worker_blink_v7.patch (22.7 KB ) - added by Michael 10 years ago.
Problem with formation update; with some warnings…
idle_worker_blink_v8.2.patch (23.0 KB ) - added by Michael 10 years ago.
Fixed bug.
idle_worker_blink_v8.patch (23.0 KB ) - added by Michael 10 years ago.
Fixed bug.
idle_worker_blink_v9.patch (24.2 KB ) - added by Michael 10 years ago.
added: select all idle workers with right click.
unitai_leave_msg.diff (1.3 KB ) - added by sanderd17 10 years ago.
idle_worker_blink_v10.patch (25.6 KB ) - added by Michael 10 years ago.
Used example from sanderd17 to not change isIdle in case Formation.idle <--> Individual.idle
idle_worker_blink_v11.patch (29.2 KB ) - added by Michael 10 years ago.
Made it compatible to the newest version of minimap.
idleWorker.png (1.7 KB ) - added by Michael 10 years ago.
put into: binaries\data\mods\public\art\textures\ui\session\icons
idleWorkerActive.png (1.7 KB ) - added by Michael 10 years ago.
binaries\data\mods\public\art\textures\ui\session\icons
idle_worker_blink_v12.patch (14.7 KB ) - added by Michael 10 years ago.
Compatible with rounded idle worker button; depends on #2421
minimap-idle-colorMask.png (3.8 KB ) - added by Michael 10 years ago.
copy to "binaries\data\mods\public\art\textures\ui\session"
idle_worker_blink_v13.patch (15.0 KB ) - added by Michael 10 years ago.
Update because #2421 was updated.

Download all attachments as: .zip

Change History (63)

comment:1 by Michael, 10 years ago

Owner: set to Michael

by Michael, 10 years ago

Attachment: idle_worker_blink.patch added

Effect is working but patch need some improvments...

comment:2 by Michael, 10 years ago

I attached a patch which make the button blink slowly if there are some idle workers.

TODO:

  • make it work when a save game is loaded (calculate the idle counter for all players)

Another thing which I don't like is how the idle counter comes from the UnitAI class (getIdleCount(player)) to session.js. Can anyone give me a hint how this can be made without using such a dirty hack (getNumberOfIdleUnits in GuiInterface.js:1712)?

One option would be to call FindIdleUnits() all the time but I think this would be bad for performance and that's why I tried to find another way.

comment:3 by Michael, 10 years ago

Keywords: patch review hint added
Summary: Make the "idle workers" button blink or change colour[PATCH] Make the "idle workers" button blink or change colour

comment:4 by Michael, 10 years ago

Milestone: BacklogAlpha 16

comment:5 by Michael, 10 years ago

Keywords: review removed

comment:6 by Michael, 10 years ago

I got some feedback from leper and sanderd17 in the IRC and will improve the patch tomorrow.

comment:7 by Michael, 10 years ago

Keywords: review added; hint removed

comment:8 by Michael, 10 years ago

Now it is working. The patch also contains some improvments of the color fade functions (easier to work with).

PS: It's not compatible to savegames which were created without this patch.

Last edited 10 years ago by Michael (previous) (diff)

by Michael, 10 years ago

Attachment: idle_worker_blink_v2.patch added

Made the code better. Thx to leper and sanderd17 for hints.

comment:9 by sanderd17, 10 years ago

Could you also display the number of idle units in the tooltip when blinking the button? This wasn't asked for, but it looks like a simple addition, as you already keep track of the number of idle units anyway.

For the code:

  • Session.js line 502 doesnt need to query the player ID. The first argument given to any GuiInterface method is the player id anyway. So that's just a lost call.
  • UnitAI shouldn't have to worry about counters too much. It's nice if it can notify the cmpPlayer when the state of a unit changed. But cmpPlayer should handle ownership changes on its own. This would make the code a lot more simple, as it only needs to work in the idle case (probably both regular and formation idle), and only ever needs to query the own player component. Having a function to query the cmpPlayer of a different player in UnitAI isn't very clear IMO. And querying the own player can already be done with a single command.

Thanks for working on this.

by Michael, 10 years ago

Attachment: idle_worker_blink_v3.patch added

Moved ownershipChanged to player; added label.

in reply to:  9 ; comment:10 by Michael, 10 years ago

Replying to sanderd17:

Could you also display the number of idle units in the tooltip when blinking the button? This wasn't asked for, but it looks like a simple addition, as you already keep track of the number of idle units anyway.

I added the label.

For the code:

  • Session.js line 502 doesnt need to query the player ID. The first argument given to any GuiInterface method is the player id anyway. So that's just a lost call.

Forgot to remove this, after I noticed that its called this way.

  • UnitAI shouldn't have to worry about counters too much. It's nice if it can notify the cmpPlayer when the state of a unit changed. But cmpPlayer should handle ownership changes on its own. This would make the code a lot more simple, as it only needs to work in the idle case (probably both regular and formation idle), and only ever needs to query the own player component. Having a function to query the cmpPlayer of a different player in UnitAI isn't very clear IMO. And querying the own player can already be done with a single command.

That's a good point. I changed that. I'm not sure if it must be added to "formation idle" because the normal idle is called when a formation is stopped. Or isn't it?

Last edited 10 years ago by Michael (previous) (diff)

in reply to:  10 comment:11 by sanderd17, 10 years ago

Replying to boeseRaupe:

That's a good point. I changed that. I'm not sure if it must be added to "formation idle" because the normal idle is called when a formation is stopped. Or isn't it?

Not anymore. In one of my recent commits, I added a real idle state for formation members. As a preparation for attacking formations.

comment:12 by Michael, 10 years ago

I think it works like it is because "UnitIdleChanged" is also called, when you move Units in formation. During my tests, I did not found any case where it was not working. If you can show me one, I'll try to fix it.

Last edited 10 years ago by Michael (previous) (diff)

by Michael, 10 years ago

Attachment: idle_worker_blink_v4.patch added

Fixed problem with smooth reset. (opacity)

comment:13 by sanderd17, 10 years ago

Reading the code again, wouldn't it be better to just include the idle units count in the GetSimulationState() code? The simulation state is also nicely cached in the Session.js.

Also, try not to hardcode classes. Define them in some template (either a boolean flag in the UnitAI, or a list of classes in cmpPlayer).

by Michael, 10 years ago

Attachment: idle_worker_blink_v5.patch added

Moved value to GetSimulationState() var.

by Michael, 10 years ago

Attachment: player.xml added

Updated player.xml but failed to change schema.

comment:14 by Michael, 10 years ago

I tried adding this to the player.js Schema, but it is not working. (template can not be parsed)

"<element name='IdleWorkerButtonClasses'>" +

"<zeroOrMore>" +

"<element name='class'><text/></element>" +

"</zeroOrMore>" +

"</element>"

What are I'm doing wrong ?

comment:15 by sanderd17, 10 years ago

I don't know what you're doing wrong, but it's impossible in any case to get the a list of elements with the same name in the js templates. The most common method is to just have a whitespace separated single string, then you only need to execute the split() command on the string to transform it to an array.

There are a lot of examples that use it, the most simple example is probable in the garrisonholder, where a class list is used to see if a unit can garrison, and one is used to see if a unit can make the building fire more arrows.

comment:16 by Michael, 10 years ago

Hmm, the problem is that the Schema for Player.js looks different than the other onces:

<a:component type='system'/><empty/>

What does that mean and where is the actual definition ?

comment:17 by sanderd17, 10 years ago

The empty part means it's empty, So you'll have to delete that, as it isn't empty any more. If deleting the empty part alone isn't enough, also delete the other thing, I don't know what it does there, as player is no system component.

EDIT: Reading the logs, leper agrees. The component is no system component, so the entire schema needs to be replaced with your new schema.

Last edited 10 years ago by sanderd17 (previous) (diff)

by Michael, 10 years ago

Attachment: idle_worker_blink_v6.patch added

Now works with class definition in player.xml file.

comment:18 by Michael, 10 years ago

Thx for the information. I missed the <Player> Tag in player.xml and tried to add the new nodes as childs of <Entity>...

comment:19 by sanderd17, 10 years ago

In IsUsedForIdleWorkerButton of UnitAI, you return arrays when the query failed. I think you should default to false instead (as the other return values are true or false).

Also, why do you need two functions IncreaseIdleWorkerButtonCount and DecreaseIdleWorkerButtonCount that are only used once, and are only 3 lines long. It's better to include it directly in the UnitIdleChanged function IMO.

For the rest, it looks fine (didn't test it again though).

in reply to:  19 comment:20 by Michael, 10 years ago

Replying to sanderd17:

In IsUsedForIdleWorkerButton of UnitAI, you return arrays when the query failed. I think you should default to false instead (as the other return values are true or false).

Also, why do you need two functions IncreaseIdleWorkerButtonCount and DecreaseIdleWorkerButtonCount that are only used once, and are only 3 lines long. It's better to include it directly in the UnitIdleChanged function IMO.

Both stuff which I used in a earlier version and forgot to change.

by Michael, 10 years ago

Attachment: savegame-0020.0adsave added

Save game for counter bug.

comment:21 by Michael, 10 years ago

The current version has (at least) one bug left:

How to reproduce:

  • load attached save game
  • move group 1 a bit down
  • group 2 will change its position
  • delete group 2

--> Counter will show 8 again but should show 7.

I think the problem is that, after group 2 has moved, it's idle state is "false". I think it should be true or shouldn't it.

A added some warnings to check that and also set the Timer for the idle state from 1s to 5s.

I have no idea how to fix that because I did not found where group 2 is told to move a bit...Any hints how to fix it?

Last edited 10 years ago by Michael (previous) (diff)

by Michael, 10 years ago

Attachment: idle_worker_blink_v7.patch added

Problem with formation update; with some warnings...

by Michael, 10 years ago

Fixed bug.

by Michael, 10 years ago

Attachment: idle_worker_blink_v8.patch added

Fixed bug.

comment:22 by Michael, 10 years ago

...

Formation.JS is calling RemoveMembers(), if the unit the ownership of the unit changes. Then the state is set to INDIVIDUAL.IDLE by UnitAI.JS. I fixed the bug by not switching to INDIVIDUAL.IDLE, if the unit is already dead.

by Michael, 10 years ago

Attachment: idle_worker_blink_v9.patch added

added: select all idle workers with right click.

comment:23 by sanderd17, 10 years ago

Instead of not switching states when the unit is dead, it would be nicer if the value of this.isIdle is't changed when switching between idle states. Therefore, the leave method should know what the next state will be. The patch I attached contains some hints on how to do that.

by sanderd17, 10 years ago

Attachment: unitai_leave_msg.diff added

comment:24 by Michael, 10 years ago

It can not switch from Indiv. idle to Formation idle (because formation must always walk in between) or can it ?

comment:25 by sanderd17, 10 years ago

It could be possible by sheer coincidence, if the formation member is already on the right position when forming the formation. But as we're on a µm precise, the chance it happens is really small. I don't know exactly if it first switches to a different state, or can directly go to the idle state.

It would still be nice to include that possibility.

EDIT: Also, requiring the nextstate array to be of length 2 is strange. It will be better to just get the last item, and check for "IDLE" there.

Last edited 10 years ago by sanderd17 (previous) (diff)

by Michael, 10 years ago

Attachment: idle_worker_blink_v10.patch added

Used example from sanderd17 to not change isIdle in case Formation.idle <--> Individual.idle

in reply to:  25 comment:26 by Michael, 10 years ago

Replying to sanderd17:

It could be possible by sheer coincidence, if the formation member is already on the right position when forming the formation. But as we're on a µm precise, the chance it happens is really small. I don't know exactly if it first switches to a different state, or can directly go to the idle state.

It would still be nice to include that possibility.

EDIT: Also, requiring the nextstate array to be of length 2 is strange. It will be better to just get the last item, and check for "IDLE" there.

Ok, I changed that.

by Michael, 10 years ago

Attachment: idle_worker_blink_v11.patch added

Made it compatible to the newest version of minimap.

by Michael, 10 years ago

Attachment: idleWorker.png added

put into: binaries\data\mods\public\art\textures\ui\session\icons

by Michael, 10 years ago

Attachment: idleWorkerActive.png added

binaries\data\mods\public\art\textures\ui\session\icons

comment:27 by Michael, 10 years ago

I made it compatible to the newest version of minimap.

Because it would look very strange, if just a part of the image would blink (the part, which was approximated using 3 the rectangles), I created a 32x32 Icon with the sleeping worker.

Also see my comment in #823.

Last edited 10 years ago by Michael (previous) (diff)

by Michael, 10 years ago

Attachment: idle_worker_blink_v12.patch added

Compatible with rounded idle worker button; depends on #2421

by Michael, 10 years ago

Attachment: minimap-idle-colorMask.png added

copy to "binaries\data\mods\public\art\textures\ui\session"

comment:28 by Michael, 10 years ago

I attached a new version of the patch which is compatible with the recent changes (rounded button image).

Depends on #2421!

How to test the patch:

  • apply colorAdd_TextureMaskMode_v3.patch of #2421
  • apply idle_worker_blink_v13.patch​ (of #2357)
  • build pyrogenesis
  • copy minimap-idle-colorMask.png to "binaries\data\mods\public\art\textures\ui\session"
  • run!
Last edited 10 years ago by Michael (previous) (diff)

comment:29 by Michael, 10 years ago

Priority: Nice to HaveShould Have

comment:30 by Michael, 10 years ago

Summary: [PATCH] Make the "idle workers" button blink or change colour[PATCH] Notify user when workers are out-of-work

by Michael, 10 years ago

Attachment: idle_worker_blink_v13.patch added

Update because #2421 was updated.

comment:31 by wraitii, 10 years ago

I don't really like how this look. Maybe it should be whiter?

comment:32 by Josh, 10 years ago

Instead of blinking, it would be much simpler (and in my opinion nicer) to just disable the idle worker button when there are no idle workers.

in reply to:  31 comment:33 by Michael, 10 years ago

Replying to wraitii:

I don't really like how this look. Maybe it should be whiter?

The color can be easily changed in the colorFade_IdleWorker() function. (binaries/data/mods/public/gui/common/colorFades.js)

in reply to:  32 comment:34 by Michael, 10 years ago

Replying to Josh:

Instead of blinking, it would be much simpler (and in my opinion nicer) to just disable the idle worker button when there are no idle workers.

But then the user doesn't get a visual notification when idle workers are there. (That's my opinion ;))

comment:35 by Josh, 10 years ago

Keywords: simple review removed

Maybe something like: disabled if idle = 0, enabled if idle > 0, flashing if idle > 10.

In regards to the patch, putting the idle classes in the player template is a little counter-intuitive. I'd prefer to just have fast response times when requesting idle workers from any class. To help visualize what I'm talking about, I'd like to keep an automatically maintained array in the player component looking something like:

uneval(idle);
// Prints ['citizen-soldier':0, 'worker':2, 'trade':0, ... ]

comment:36 by Josh, 10 years ago

Milestone: Alpha 16Alpha 17

comment:37 by Josh, 10 years ago

Owner: changed from Michael to Josh

boeseRaupe: Do you still have time for this? As I haven't seen you around for awhile, I'll try to finish this.

comment:38 by Itms, 10 years ago

Hello Josh, do you think you'll be able to finish this today? We're supposed to go into feature freeze during the meeting.

comment:39 by Itms, 10 years ago

Milestone: Alpha 17Alpha 18

Pushing because of the feature freeze.

comment:40 by Itms, 9 years ago

Milestone: Alpha 18Alpha 19

comment:41 by mimo, 9 years ago

Josh are you still working on it ? or should we move it to backlog ?

comment:42 by Josh, 9 years ago

Milestone: Alpha 19Backlog

I think I have a partial fix somewhere, but I'm not working on it at the moment.

comment:43 by Imarok, 5 years ago

Component: UI & SimulationIn-game UI

Move tickets to In-game UI as UI & Simulation got some sub components.

Note: See TracTickets for help on using tickets.