Opened 13 years ago

Closed 11 years ago

Last modified 11 years ago

#799 closed enhancement (fixed)

[PATCH] Add "back to work" button

Reported by: MiniMe Owned by: Itms
Priority: Must Have Milestone: Alpha 15
Component: Core engine Keywords:
Cc: againsttcpa84@… Patch:

Description

Hi!

What I'm missing the most in 0 A.D. is the possibility to send units (citizen-soldiers) back to their last assigned work (i.e. resource gathering) after using them for battle. It's a hassle to reassign them manually each time, so this would be a great improvement, especially because in 0 A.D. it's "normal" to use units for battle and resource gathering.

Btw, there is such a button in other games too (e.g. "Empires:DOTMW"), where citizens are able to fight...

Attachments (6)

patch_backtowork.diff (5.8 KB ) - added by Itms 11 years ago.
patch_backtowork.2.diff (5.9 KB ) - added by Itms 11 years ago.
patch_backtowork.3.diff (7.5 KB ) - added by Itms 11 years ago.
patch_backtowork.4.diff (10.0 KB ) - added by Itms 11 years ago.
patch_backtowork.5.diff (6.8 KB ) - added by Itms 11 years ago.
patch_backtowork.6.diff (6.7 KB ) - added by Itms 11 years ago.

Download all attachments as: .zip

Change History (30)

comment:2 by michael, 13 years ago

This button can go in the center panel of the Building's UI, beneath the portrait, and can toggle between a "Bell" icon and a "Bell with arrow" icon.

comment:3 by Darth_Malloc, 12 years ago

Owner: set to Darth_Malloc
Status: newassigned

comment:4 by historic_bruno, 12 years ago

Owner: Darth_Malloc removed
Status: assignednew

comment:5 by Darth_Malloc, 12 years ago

Owner: set to Darth_Malloc

comment:6 by historic_bruno, 11 years ago

Owner: Darth_Malloc removed

Please stop assigning tickets to yourself if you're not going to do some work on them.

comment:7 by Itms, 11 years ago

Keywords: review patch added
Summary: add "back to work" button[PATCH] Add "back to work" button

I've written a patch which implements this behaviour, as well as the corresponding UI elements.

by Itms, 11 years ago

Attachment: patch_backtowork.diff added

comment:8 by sanderd17, 11 years ago

In which cases is it supposed to work? I've tried a number of cases, but I can't get them back to work.

It should in any case be saved over any move, fight and garrison commands.

comment:9 by Itms, 11 years ago

This is strange, it works perfectly for me after moving, fighting and garrisoning.

Last edited 11 years ago by Itms (previous) (diff)

comment:10 by sanderd17, 11 years ago

Can you describe how it's supposed to work? I'm probably doing something wrong. But if it's too difficult, it's a usability issue.

comment:11 by Itms, 11 years ago

I really don't know what you could do wrong...

When a worker is gathering from a resource supply, give him an order (e.g. go somewhere, or garrison, or attack an enemy unit). If you click on "Back to work" button, the worker should go back to the supply he was gathering from and continue his work.

P.S. I reverted to the revision r13868 and applied my patch and it still works.

Last edited 11 years ago by Itms (previous) (diff)

comment:12 by Itms, 11 years ago

I don't know if you succeeded, but in the meantime I added a condition to prevent the workers from going back to a "collecting treasure" work, which doesn't make any sense.

See also #2136.

by Itms, 11 years ago

Attachment: patch_backtowork.2.diff added

comment:13 by Itms, 11 years ago

Milestone: BacklogAlpha 15

Still don't know whether you succeeded... (sorry for spamming you with that !)

I opened a topic here to get feedback from more people : http://www.wildfiregames.com/forum/index.php?showtopic=17643

I've added the back-to-work feature to fishing boats (they don't share the class 'Worker' I used in my patch), but I doubt it is useful since fishing boats can't do anything but fishing, so they never have to go back to work. Nonetheless, if you think it could be helpful I shall upload the alternative patch.

I also change the Milestone to Alpha 15, since I forgot it when I proposed the patch.

comment:14 by mimo, 11 years ago

I've not run the patch, just looked at the code, but here are some comments :

  • I do not understand how it can work with garrisoned units as you never ungarrison them, but may-be I missed something ?
  • You only deal with gatherers, but there are other workers we would like to go back to their tasks (builders or traders for example). I guess it would be simpler and more generic to bakup the UnitAI OrderQueue and restore it when pressing the backToOrder button.
  • The last problem is more a design question to be discussed: if I send an army to attack the ennemy base and the ennemy attacks my base at the same time, once I've repelled him, I only want units from my base to go back to previous orders, not those attacking the ennemy base. So we should have a way to differentiate orders which we want to be able to go back-to-work (usually non fighting units and defenders of our base), and those which we do not want (usually attacking army). May-be having a hot-key which when pressed while giving a forced order would indicate that we want to bakup the present orders before erasing them with the forced order ? or any better idea ?

comment:15 by Itms, 11 years ago

Owner: set to Itms

Thanks for the feedback.

  • The garrisoning system is controlled by another component than ResourceGatherer, that's why I don't have to care about un-garrison process. Just try the patch (if it works for you) and you'll see it works.
  • I'll think about that, you're totally right.
  • Hmm, I think that there is no problem since you just have to select only the units in your base and send them back to work, this won't affect the attacking units in the enemy base...

comment:16 by mimo, 11 years ago

OK, your implementation is different of what I had in mind: here you have to manually ungarrison your units, then select them and send back to their previous work, while I was thinking of a global button which would automatically send all units back to work without having to ungarrison or select them. So forget about my comments 1 and 3 which do not apply to the way this patch works.

I've now tested it, and it works for me.

comment:17 by Itms, 11 years ago

I upload a new version of the patch :

  • All Gathering, Trading and Building works are now memorized. The new memorizing implementation is more generic and allows units to go back to a work they were assigned to, even if they hadn't had the time to begin it.
  • mimo's feedback first item gave me an idea : I added a button called "Unload All Units and Send the Workers Back to Work" to the GarrisonHolder entities. When pressing this button, the garrisoned workers are ejected and ordered to go back to their work, while non-workers units are ejected to the rally point.

by Itms, 11 years ago

Attachment: patch_backtowork.3.diff added

comment:18 by Itms, 11 years ago

New version of the patch, based on some leper's and redFox's comments on IRC.

  • The Back To Work button now only shows up if the selected unit remembers any work
  • More generally, the Back To Work behaviour applies to any unit able to remember a work, even if it's not a Worker or Trader unit
  • When a task is complete, the finished work is forgotten
  • ReturnResource is now a remembered work, but only if it has been ordered directly by the player (not if the unit is automatically returning what it gathers)
  • Promoted units remember their work

by Itms, 11 years ago

Attachment: patch_backtowork.4.diff added

comment:19 by Itms, 11 years ago

Removed the "Unload All and Back To Work" in this patch.

This behaviour will be implemented in Town Bell (#2154) as we shall garrison units and get them back to work later only in case of emergency/attack.

by Itms, 11 years ago

Attachment: patch_backtowork.5.diff added

comment:20 by sanderd17, 11 years ago

It works now (for some reason), and I'm quite happy with it. The only problem is when you select multiple units.

For example, say unit A can return to work, but unit B has no previous work.

  • If you first select unit A, and than (by holding shift) unit B, the back to work button is active, and unit A returns to work as expected when you press the button.
  • If you reverse the order of selection (first B, than A), the back to work button isn't active.

When you have a group of units, it's enough that the first one is able to return to work to get the button. IMO, this is a bug, the button should be active if there is any unit that can return to work. Can you investigate if it can be solved (it might be a bug already present in 0 A.D.

On your code, it looks very good. Just line 2142 of input.js, I'd make that more simple by returning state && state.unitAI && state.unitAI.lastWorkOrder instead of having the if statement.

Thanks for working on it.

comment:21 by Michael, 11 years ago

Cc: againsttcpa84@… added

in reply to:  20 ; comment:22 by Itms, 11 years ago

Replying to sanderd17:

Just line 2142 of input.js, I'd make that more simple by returning state && state.unitAI && state.unitAI.lastWorkOrder instead of having the if statement.

Fixed.

Nonetheless, I can't fix the problem you pointed out. It's actually a bug, in unit_command.js line 1011, the function updateUnitCommands takes the entity state of the first selected unit, which is used by getEntityCommandsList on line 1030. But since the code here can't access the components of an entity, we would need to compute the entity state of every selected unit to make the back to work button show up, wouldn't it be a real waste of time/memory ?

Moreover, the same problem occurs when selecting multiple buildings, when some can hold a garrison and some can't. 

by Itms, 11 years ago

Attachment: patch_backtowork.6.diff added

in reply to:  22 comment:23 by sanderd17, 11 years ago

Replying to Itms:

Nonetheless, I can't fix the problem you pointed out. It's actually a bug, in unit_command.js line 1011, the function updateUnitCommands takes the entity state of the first selected unit, which is used by getEntityCommandsList on line 1030. But since the code here can't access the components of an entity, we would need to compute the entity state of every selected unit to make the back to work button show up, wouldn't it be a real waste of time/memory ?

Moreover, the same problem occurs when selecting multiple buildings, when some can hold a garrison and some can't. 

As it only happens for the selected units, I think it can be optimized and cached, so the waste of time and memory doesn't count if you do it right.

It's an issue, but not your issue. Can you make a new bug report for it? I'll look into committing your patch.

comment:24 by sanderd17, 11 years ago

Resolution: fixed
Status: newclosed

In 13987:

Add a back to work button. Patch by Itms. Fixes #799

comment:25 by sanderd17, 11 years ago

Keywords: review patch removed
Note: See TracTickets for help on using tickets.