[PATCH] Improve the "back to work" behaviour

[Itms]

This patch allows units to remember a queue of work orders, and not a single one (useful for instance when constructing several buildings).

It also reduces the code and makes the button show up only when it's necessary (thanks mimo !)

I am currently also solving the problem with formation members. For now the attached patch works fine for female citizens and alone workers.

[mimo]

Still the new changes have the problem that we loose the queue. In addition, there are some orders like ReturnResource which may be an order we want to remember (in case it was given by the player), but it may also be a sub-order of the gather order, in which case we rather want to remember the gather order.

So I wonder if it would not be simpler to replace the changes in UnitAi (finishOrder, pushOrder and your new ones) and only have in ReplaceOrder something like: if the added (forced) order is a "not work" order, and the first forced order in the queue is a work order, then put all the previous queue in this.lastWorkOrder. And if the added order is a "work" order, reset this.lastWorkOrder.

Futhermore, the gui only needs a bool to know if the unit has some WorkOrder or not, so it's not necessary to pass the full order (even more if we implement the queueOrder for lastWork) to the gui in GuiInterface.

[Marcio]

All units inside a building go to back and then they must y ignore rather rally point and try to back to the work.

[Itms]

Thanks mimo,

This is a patch which implements the back-to-work order queue.

It should work fine, please tell me if anyone experiences any problem !

[mimo]

Thanks for the new patch. The queues works fine now. I've still two comments concerning the UnitAI part: the BackToWork function can be simplified a lot by using the AddOrders function, and as I said in my previous comment, I think spreading the logic inside two functions (ReplaceOrder and FinishOrder) is not necessary.

I attach a slightly modified version of your patch with these two changes in UnitAI. let me know what you think.

[Itms]

I'm sorry I don't have time to test your patch right now, but I don't really understand how your patch could work (there's at least a problem with i0 in your loop in replaceorder)

More generally, I think the way your patch works is far less understandable (but maybe it's because I worked myself on my patch ;) ) ; could you make sure it works in any case ? For instance, queue building orders with multiple builders, take one of them somewhere else, make him go back to work, and once all the buildings are finished, check the button doesn't show up for him (I had a lot of trouble with that, which prevented me to use addOrders in backToWork).

Thanks for your review, anyway =)

EDIT : btw, when you talk about "slight" modifications, I don't really agree ;)

[mimo]

Replying to Itms:

I'm sorry I don't have time to test your patch right now, but I don't really understand how your patch could work (there's at least a problem with i0 in your loop in replaceorder)

Could you be more explicit ? I don't see the problem (but it's true that I have not tested the case where we change order while cheering).

More generally, I think the way your patch works is far less understandable (but maybe it's because I worked myself on my patch ;) ) ; could you make sure it works in any case ? For instance, queue building orders with multiple builders, take one of them somewhere else, make him go back to work, and once all the buildings are finished, check the button doesn't show up for him (I had a lot of trouble with that, which prevented me to use addOrders in backToWork).

Concerning the code lisibility, I fully agree that it may be a question of taste: if the code size is small (about 10 lines here), I prefer to have it all in one function instead of scattered in several functions. I think it is much simpler to maintain or even change it in the future. But certainly others have different opinions.

For the other point, my modifications should rather be seen as a work-in-progress proposition. I've done limited testing, although I've checked that queued orders worked in various cases. The part which is missing is how to deal with units in a formation: when we select a unit which was in a formation, it is removed from the formation while the queued orders were applying to the formation. But I think your patch also does not take it into account.

On a side note, in your BackToOrder function, there is a potential problem if ordering back to work while cheering. The first order (cheering) is already being processed, and you try to process it again.

Thanks for your review, anyway =)

EDIT : btw, when you talk about "slight" modifications, I don't really agree ;)

[Itms]

Okay, I tested it and now I understand ! I was totally unable to figure out how it worked because it's completely different of what I had in mind : you remember works when other orders are given while I remember works when they are ordered. I had not understand it when you explained it in your first comment, I apologise. 

What I like in your idea is above all the fact that the back to work button will show up less frequently.

So, thanks again for your help, I attach a new version of the patch. The problem with i0 may not be an error, but a different point of view ; I think we should slice the orderQueue from i (i.e. the first work order) and not from i0.

I fattened a bit backToWork because I think the back-to-work order should replace the queue, not just push work orders at its end.

I also reordered the functions in UnitAI, grouping backToWork with other workorders functions.

Next step : formation orders.