Improve production prioritisation

Bug #861761 reported by Nasenbaer
38
This bug affects 5 people
Affects Status Importance Assigned to Milestone
widelands
Fix Released
Medium
Unassigned

Bug Description

the default target quantity for wares should be 0.
it can be *very* annoying, if you lost your headquarters and have an empty warehouse and the only thing the metalworkshop produces are tools you do not need in the next half hour - but simply no felling_axe for your lumberjack...

Related branches

Revision history for this message
Victor Pelt (victor-pelt) wrote :

actually i think there should be a difference between need and need. target quantity should be produced at a lower priority than required goods.

Revision history for this message
SirVer (sirver) wrote :

acually, there should be a way to order some wares to be produced immediately (e.g. 5 axes). There is a fantastic blueprint by astuur which incorporates a UI design for this: https://bugs.launchpad.net/widelands/+bug/796644

Revision history for this message
Astuur (wolfsteinmetz) wrote :

Same thing when playing the "Hardcore" start condition.
In addition to the "flash orders" it should also be possible to set the default_target_quantity=
to "0" in the game.
For soldiers, setting default_target_quantity= (all you have in your warhouses)
could also help with the old problem that it is so hard to
control how much of your metal goes into new soldiers, and how much into the
training of existing ones.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

What would the consequences be for wares with target quantity 0 when I run out of them. Say I build a lot of farms, and are able to staff some of them and use the scythe(s) I started with to get some more farmers. But I still have at least one unoccupied farm, which will not get a worker until I produce a new scythe for the farmer to use. Will the smith do this at some point or will he ignore scythes because the target quantity is set to 0?

Revision history for this message
Nasenbaer (nasenbaer) wrote :

target quantity does *only* mean "number that should be freely available in the economy" - so as long term "number of that ware that should be stored" - in case of a a target quantity of 0 forscythes with existing request for scythes the scythe would of course be produced anyways. so setting default target quantity to 0 does only change the behaviour of the production sites that way, that they do only produce a ware if it is needed at the moment, or when the user sets the target quantity to a value higher than 0 (and in that case until the number of that ware is freely available in the economy). Hope that clearede it up :)

Nasenbaer (nasenbaer)
Changed in widelands:
assignee: nobody → Nasenbaer (nasenbaer)
Revision history for this message
Nasenbaer (nasenbaer) wrote :

Fixed in rev. 6296 - I playtested and it makes indeed a big difference :), if you are in very need of a specific tool, but only own a few materials to produce wares.

Changed in widelands:
status: Confirmed → Fix Committed
Revision history for this message
Angelo Locritani (alocritani) wrote :

@Nasenbaer(#6 ):
I think it can be a bit dangerous for new players: all metalworks' programs are like skip unless <tool> needed.

So tools would not be produced unless the player rises the initial target quantity. I fear new players will simply ignore the configure economy window and remain without tools.

Revision history for this message
Angelo Locritani (alocritani) wrote :

sorry: just overlooked your sentence: "in case of a a target quantity of 0 forscythes with existing request for scythes the scythe would of course be produced anyways." - please ignore my previous post.

Revision history for this message
Angelo Locritani (alocritani) wrote :

Previously, only wares for which a rule like "skip unless economy needs" or "produce if economy needs" exists were in configure economy window.
Now all wares are displayed.
screenshot attached

Revision history for this message
Nasenbaer (nasenbaer) wrote :

Jepp. Absolutely correct. Seems I should have taken a closer look to the behaviour I changed. Thanks for noticing this problem, Angelo! :)

Fix for the fix is in bzr rev 6304

Revision history for this message
_aD (ad-simplypeachy) wrote :

Using revision 6322 any wares which have target quantity 0, for example the fishing rod, are not created by the toolsmithy if they are required, which is a regression.

Steps to reproduce (Empire):
Build a toolsmithy and two fisher's houses
Once completed, build a third fisher's house. The toolsmithy will not create the required fishing rod until the target quantity of the fishing rod is >0

Revision history for this message
_aD (ad-simplypeachy) wrote :

Also I'd like to address the original comment for this change. I do not want to have to wait for my toolsmithy to product a fishing rod and wait for it to be transported to my HQ before I can get my fisherman. This is causing an unnecessary loss of efficiency.

I think giving "required" wares a priority over "needs to reach target" is a great idea, but in my opinion at least one of each tool should be kept in stock at all times.

Revision history for this message
Astuur (wolfsteinmetz) wrote :

I hope I make sense:
The minimal storage quantity for wares could not be set to 0 before and now it can.
That is good for the situations described above.
However the default for the minimal storage wares at a start of the game must not be 0,
because you run into some problems. Instead, the user will have to set it to 0 manually when he needs to.
As an example, I could not build some houses because the kiln would not produce grout, although everything needed was
available (grout minimal storage value was set to 0)

Revision history for this message
SirVer (sirver) wrote :

I can confirm #11 - this is pretty critical: requirements do not seem to be checked if target quantity is zero.I set this to critical and build 17 because of this.

 Also, I agree with #12 that the default target quantity should be 1 - the user can set it to zero if he needs too quickly enough. So +1 for making default target quantity.

Changed in widelands:
status: Fix Committed → Confirmed
importance: Medium → Critical
Revision history for this message
Nasenbaer (nasenbaer) wrote :

sounds like a if (target_quantity) check somewhere, where it should not be.

Unfortunally I am very busy at the moment and can't afford the time to search for the problem. And even if I found the problem, the fact that this would be the third fix already shows, that the *new fix* should be tested a bit longer. So anybody willing to take a look at this? If not, I vote for reverting my changes and retargeting this bug for the time after build17.

Changed in widelands:
assignee: Nasenbaer (nasenbaer) → nobody
Revision history for this message
SirVer (sirver) wrote : Re: [Bug 861761] Re: Make default target quantity 0

Right. Let's do this and investigate later on. Peter, can you back the
changes out? You know best when you made them.

Revision history for this message
Nasenbaer (nasenbaer) wrote : Re: Make default target quantity 0
Changed in widelands:
milestone: build17-rc1 → build18-rc1
Revision history for this message
Nicolai Hähnle (nha) wrote :

It doesn't seem like this bug is really critical, is it?

As a matter of fact, I don't even agree that having target quantities equal to 0 by default is a good idea, because it's probably not the reasonable setting for new players. The default is that you do want tools in reserve, so that you don't have to wait around for them to be produced when you build a new building.

If you want to fine-tune the behaviour of the metalworks, then I would consider you to be advanced enough that you can change those settings yourself.

Changed in widelands:
importance: Critical → Medium
Revision history for this message
_aD (ad-simplypeachy) wrote :

I agree with #18. However the bug's title and description do not really match - the description makes me think some sort of automatic priority is needed.

If you have 0 axes and 0 hammers, and there is demand for one axe and 10 hammers, one of each should be created before the other nine hammers are supplied.

Revision history for this message
SirVer (sirver) wrote :

This bug was set to critical by me when this feature was implemented - because of #11. The change has been backed out since, so this is no longer critical, just nice to have.

_aD (ad-simplypeachy)
summary: - Make default target quantity 0
+ Improve production prioritisation
Revision history for this message
Frank Pieper (frank-pieper-1) wrote :

Buttons for ALL and INVERSE resp Buttons for Rows resp a Way to draw Rectangles to select Items to modify.
 Aside of INC/DEC a SET (to Value) would be reasonable.

SirVer (sirver)
Changed in widelands:
milestone: build18-rc1 → build19-rc1
Revision history for this message
fk (fredkuijper) wrote :

#5 "in case of a a target quantity of 0 forscythes with existing request for scythes the scythe would of course be produced anyways."

That would actually undermine the reason for this option. The benefit of option zero is that it can stop soldier production, as a last resort against eternal soldier requests. This would enable the arena to be functional, and only train soldier when needed, but without constantly creating new units.

#7 "I think it can be a bit dangerous for new players"

New players won't know that there is an economy menu, or be wise enough not to touch it. (But if they want to play with fire ...)

Question: why does it take over two years to change 1 to 0? It is as simple as it can be and players are free to ignore it. Experienced players, however, will be very happy with this new option.

Revision history for this message
_aD (ad-simplypeachy) wrote :

See comment 15 - this caused game-breaking bugs so will need someone with time to fully test to make sure it works. This shows that it is not a simple "change 1 to 0" as even seemingly simple changes can have calamitous effects! I have no use for a 0 target quantity. If I do not want soldiers I stop producing them.

Revision history for this message
fk (fredkuijper) wrote :

#23

#15 refers to #11:

"Using revision 6322 any wares which have target quantity 0, for example the fishing rod, are not created by the toolsmithy if they are required, which is a regression."

That's precisely what it is supposed to do, if I see it correctly.

Revision history for this message
SirVer (sirver) wrote :

> That's precisely what it is supposed to do, if I see it correctly.

No. target quantity means the target of spares that are available in your warehouses. If you set it to zero, everything should be still produced when requested. Stopping one ware of being produced is not supported right now (except for stopping buildings of course).

Revision history for this message
fk (fredkuijper) wrote :

From that perspective it is a clean solution, and I can understand very well that many people want it in that way. But where I live they say "to catch two flies in one beat." There have been multiple complains about the inability to stop soldier production and with the current logic there is no other way to handle that than by closing all buildings with a request for soldiers capability. I really wonder how that problem is ever going to be solved. Allthough it is perhaps cleaner to handle the zero value in the way that you described above, there might be a greater gain if it can also be used to get the soldier production under control.

Revision history for this message
fk (fredkuijper) wrote :

Apologies, that was an unnecessary repeat of arguments.

Revision history for this message
SirVer (sirver) wrote :

Setting to incomplete for bug sweeping.

Changed in widelands:
status: Confirmed → Incomplete
Revision history for this message
meitis (meitis) wrote :

Hi,

impletented this:

default target quantity remains unchanged, but it is now
possible to change the target quantity to 0. ProductionSites
will only start producing if a unfullfilled request is in
the economy.

Though not tested yet, also I am pretty sure that two "concurring" production sites would currently both start producing, leading to 1 ware produced too much. Fixing that would require to do some Supply magic, creating a supply for a unproduced ware, that seemed to complicatedfor the moment.

Leaving this for discussion. If you like the approach, I can look into the Suppply magic

Revision history for this message
meitis (meitis) wrote :

ok, the economy code is a bit more complex than a first glance revealed to me ;)

but, I tested and debuged it, updated my branch ... and now it really works!

My test:
start a new game as barbars, set bread_paddle target quantity to 0, build metalworks and 2 bakeries. Wait until all is done and equiped. You should have 0 bread_padles in your headquater and now bakers neither. Now build a 3rd bakery ... and guess what, the metalworks produces exactly one bread_paddle :)

the branch to merge for your own testing:
lp:~meitis/widelands/bug861761

Revision history for this message
GunChleoc (gunchleoc) wrote :

I will do some compiling & testing to double-check that everything works.

For now, I noticed in the commits that you have some TODO comments still left - are these things still to do, or are they solved? While I'm working on the branch, I always use the keyword NOCOM for things that need to be solved before it gets merged, so I can grep for it separately from the TODO keyword.

I see that you have already noticed our Codecheck stuff :) If you want to run it separately from compile.sh, you can use

cmake/codecheck/CodeCheck.py src/* | grep -v "src/third_party"

Revision history for this message
meitis (meitis) wrote :

One TODO was behind my current knowledge. I changed it to what I believe needs to be done, but this is actually independed from this change => would leave it in, maybe create a new bug/task out of it.

the second TODO still needs to be done: "find nearest warehouse" (the current code works, with 1 "warehouse" / the headquater perfectly well, with multiple, well, it works, but not necessarily as efficient as it could be) ... I think I somewhere saw some code about this (find a close by warehouse). If you know it just right away, could you help me where to find it? Otherwise I will search it myself.

Revision history for this message
meitis (meitis) wrote :

found the function find_closest_warehouse ;)

but I want to think a bit more about get_stock_policy before I update my branch

Revision history for this message
GunChleoc (gunchleoc) wrote :

Try src/ai/defaultai.cc, line 2090, unless that's the one you found already.

Revision history for this message
meitis (meitis) wrote :

used Economy::find_closest_warehouse that uses a star algorithm as path finding which is more efficient.

also commented in the code: its ok to ignore stock policy.

tested now with headquater and additional 1 warehouse. it seems to work, the bread_paddle is send to the warehouse closest to the bakery that needs the worker.

I think I'm done, you can test and review

Revision history for this message
GunChleoc (gunchleoc) wrote :

Good news :)

Can you please submit a merge request then, so we can get a diff?

GunChleoc (gunchleoc)
Changed in widelands:
status: Incomplete → Fix Committed
GunChleoc (gunchleoc)
Changed in widelands:
status: Fix Committed → Fix Released
Revision history for this message
GunChleoc (gunchleoc) wrote :

Fixed in build19-rc1.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Related blueprints

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.