No training in camp for Empire

[spth]

A few days ago, I started a game as Empire (red player in Lesser Ring 1.2 map).
Yesterday I was finally able to build a training camp.

As soon as it was finished, 4 soldiers went in there (I had set the capacity to 4). No training happened, since there were no helmets for a while.

The moment helmets arrived, the 4 soldiers left the camp. Now, I no soldiers are going to the camp. Though there are plenty of soldiers sitting in the warehouses.
I tried reducing the capacity to 0, the increasing it again a few times, I tried reloading the savegame. Still, no soldiers go to the training camp.

On the other hand, I see well-trained Atlantean soldiers from AI players, so traning must work for them. Also my Colosseum is working fine.

P.S.: Both at game start and yesterday, I was using widelands from then-current git.

P.P.S.: Savegame: http://www.colecovision.eu/stuff/widelands-no-training/PkK%202.wgf

[hessenfarmer]

At current git there is a new selection algorithm for soldiers implemented. Which helps to select the best suited (most trained) soldiers to be sent to a trainingsite. Unfortunately this takes a while to settle down after there were to many failed attempts to train. So I assume just waiting a few (around 20 secs) after changing the capacity should fix this. However this is not intuitive and should be cured imho.

[Niektory]

I loaded your save with a6d0c10. I observed:

• 4:50:48 - Save time. The Training Camp is full of wares, has no soldiers inside and 4 vacant slots. There are evade-trained soldiers in the Headquarters that could be trained in the Training Camp.
• 4:51:51 - One rookie leaves the Barracks and starts heading towards the Training Camp.
• 4:52:13 - The rookie enters the Training Camp and starts getting trained.
• 4:53:00 - 3 evade-trained soldiers leave the Headquarters and head towards the Training Camp to fill the remaining 3 slots.

The soldier selection by training sites was changed in 1a00367. I agree that the delays are way too long and I complained about it before. Not only that, in this case it actually made soldier selection worse, taking only a rookie when evade-trained soldiers were available and the Training Camp was full of wares. It seems it's still not working well.

This needs to be fixed for Build 21.

Pinging @tppq

[spth]

The 4 soldiers that had been in the camp and were sent out the moment the helmet arrived were also all evade-trained.

[tppq]

Pinging @tppq

Thanks for that.

I agree that the delays are way too long and I complained about it before.

I remember this conversation well. I shortened the timeouts then. Maybe I messed up, and the modification did not go in. I'll investigate.

[tppq]

Maybe I messed up

Not like this, is a bug.

[tppq]

The moment helmets arrived, the 4 soldiers left the camp.

The bug triggered ~1 work cycle before this happened. I can fix the immediate problem.

Question to all: If the selective code does not get soldiers, is it then always better to ask for anybody, instead of having breaks like this? If so, can do that in addition to fixing the bug.

[tppq]

What should the timeout be? 20 seconds mentioned.

[tppq]

What should the timeout be? 20 seconds mentioned.

[hessenfarmer]

the problem will always be if the capacity of a training site will be increased AND you don't see soldiers leaving the warehouse while you know they are there you will be confused. So we should keep this very short anything more then 5 sec will cause confusion I believe.

[Noordfrees]

We need to allow a reasonable threshold here because I'm not sure without checking how often the economy recalculates its supply-request-allocation. Too short and the trainingsite's patience will timeout before the economy even had the chnace to try to send a soldier, which would be a worse bug than a few seconds enforced idleness. Perhaps 10 seconds would be reasonable but now below.

[Noordfrees]

Question to all: If the selective code does not get soldiers, is it then always better to ask for anybody, instead of having breaks like this? If so, can do that in addition to fixing the bug.

Gradually lowering the requirements would be best IMHO

[tppq]

Gradually lowering the requirements would be best IMHO

I agree. This was a bug, and the code entered into weird state. The positive side is that it recovered.

I made an unbelievably stupid thing just moments ago. I decided to force rebuild once more and run the testset before pushing. Now, instead of rm -Rf build, I accidentally did rm -Rf src, without committing first. Need to rewrite. Sorry for the delay.

We need to allow a reasonable threshold here because I'm not sure without checking how often the economy recalculates its supply-request-allocation.

Would it be possible to get a call back after the check? I have assumed that it is not easily doable. If it would be easily doable, then there would be no need for timeouts.

the problem will always be if the capacity of a training site will be increased AND you don't see soldiers leaving the warehouse while you know they are there you will be confused.

I can add side-effects to current interactions if that is a good idea in your opinion.

Simplest would be that user interaction causes everybody to be accepted, arriving soldier re-enables the pickiness.

Would you be happy if any user interaction would turn off all selections, or is increasing capacity special? Eventually we should have the buttons as in #3827 . My drawings are really lousy and therefore I hesitate to self-assign, especially with freeze close. I gladly knit the logic with the new interactions, if the UI somehow appears. If Hessenfarmer is okay with the above drafted side effect, I am also okay with it.

[Noordfrees]

Would it be possible to get a call back after the check? I have assumed that it is not easily doable. If it would be easily doable, then there would be no need for timeouts.

Nope, that would require quite a lot of refactoring in the economy-request backend.

Would you be happy if any user interaction would turn off all selections, or is increasing capacity special? Eventually we should have the buttons as in #3827 .
My drawings are really lousy and therefore I hesitate to self-assign, especially with freeze close. I gladly knit the logic with the new interactions, if the UI somehow appears.

Any new buttons will have to wait until after b21 because string freeze is less then 25 hours away. But then a Prefer Any button to always accept the first-best soldiers would be the best solution IMHO.

[tppq]

Too short and the trainingsite's patience will timeout before the economy even had the chnace to try to send a soldier, which would be a worse bug than a few seconds enforced idleness.

The worst that a too short timeout would give is that a trainingsite would take in a less trained soldier than could have been possible. World does not end there.

because string freeze is less then 25 hours away

The freeze is our own choice. We can still change that, if we want, I guess? Who chooses these now? Is GunChleoc around or having a WL-vacation?

[tppq]

I made an unbelievably stupid thing

Because of circumstances, I did not test the rewrite yet, will have to wait for tomorrow ;-)

[Noordfrees]

Too short and the trainingsite's patience will timeout before the economy even had the chnace to try to send a soldier, which would be a worse bug than a few seconds enforced idleness.

The worst that a too short timeout would give is that a trainingsite would take in a less trained soldier than could have been possible. World does not end there.

Also true. Still I think a few seconds more don't really matter.

because string freeze is less then 25 hours away

The freeze is our own choice. We can still change that, if we want, I guess? Who chooses these now? Is GunChleoc around or having a WL-vacation?

I haven't seen Gun for perhaps three weeks, so I declared myself the manager of the release in her absence. And I already declared the schedule and I won't go back on my word unless a really critical issue requires it.

I made an unbelievably stupid thing

No problem, we all do it now and then :)

[tppq]

The worst that a too short timeout would give is that a trainingsite would take in a less trained soldier than could have been possible. World does not end there.

Also true. Still I think a few seconds more don't really matter.

I do not see a problem, but of course would be happier if everybody here are content with the new functionality.

The attached branch probably fixes the issue. Note that the problem has already happened in the savegame of PkK, so its behavior is still what it is.

Fulfilling the request of @hessenfarmer :

if the capacity of a training site will be increased AND you don't see soldiers leaving the warehouse while you know they are there you will be confused.

is doable in the way drafted above, but unfortunately has to wait until Thursday evening at least. Hopefully this is causes no problems.

[hessenfarmer]

wow so much conversation while I was just trying to solve some things out. Ok on topic:

1. I think the hack proposed by tppq (resetting on user interaction in the soldiers tab) would sufficiently mask the thing so it shouldn't be noticed by the majority of players. So this could be the solution for now. Just make sure you don't reset the soldiers while some input sliders are changed.
2. I think everything already reported in the forum or here which is not trivial (can be solved by the average player) should be considered a bug worth fixing. So I'd rather vote to have longer time to bugfix. This could include small string changes if necessary and they can't be circumvented like in this case.
3. the release should be aimed to produce as less repeating complaints as possible.
4. I really appreciate you tokk the role of release manager for this time.

[Noordfrees]

I think the hack proposed by tppq (resetting on user interaction in the soldiers tab) would sufficiently mask the thing so it shouldn't be noticed by the majority of players. So this could be the solution for now. Just make sure you don't reset the soldiers while some input sliders are changed.

+1 resetting on kicking soldiers and changing capacities should be enough for now

I think everything already reported in the forum or here which is not trivial (can be solved by the average player) should be considered a bug worth fixing. So I'd rather vote to have longer time to bugfix. This could include small string changes if necessary and they can't be circumvented like in this case.

https://www.widelands.org/wiki/ReleasingWidelands/: "only minimal invasive bug fixes. That is if a bug can be fixed in a dirty-but-minimal-invasive change and in a correct way, the one that doesn't touch on any other code should be preferred and the correct way should go into a feature branch."

If we will really need string tweaks (but we won't need any for this feature, right?), any tweaks that are necessary now could be committed to master already while the code changes are reviewed in good time. However announced string freeze means string freeze so no changes to texts after tomorrow evening 22.00

the release should be aimed to produce as less repeating complaints as possible.

Then we'd have to fix all 401 issues before making a release. There will always be something users complain about, and every new feature will get people saying it could be improved like this, no, like that, or…
Let us not forget that this feature here – even if it may seem unintuitive to some users sometimes – is still always an improvement over the previous algorithm.

[Niektory]

Let us not forget that this feature here – even if it may seem unintuitive to some users sometimes – is still always an improvement over the previous algorithm.

Currently not true. In the reported case it performed worse than the previous algorithm in every way (worse selection, worse speed).

Also I'm a believer in the principle of least surprise. Things should work how users expect them to work. There are exceptions but it's a good rule of thumb.

Keeping that in mind, I believe the delays should be very short, even if it makes the selection not always ideal. Users don't expect it to be ideal, but they expect it to be fast. So I'd say make it fast-and-decent first, and perfect the precision later (and speed will be less important when the buttons are added).

Would you be happy if any user interaction would turn off all selections, or is increasing capacity special? Eventually we should have the buttons as in #3827 . My drawings are really lousy and therefore I hesitate to self-assign, especially with freeze close. I gladly knit the logic with the new interactions, if the UI somehow appears. If Hessenfarmer is okay with the above drafted side effect, I am also okay with it.

I think I can make decent-looking icons for buttons and such if that's any help.

[tppq]

Currently not true. In the reported case it performed worse than the previous algorithm in every way (worse selection, worse speed).

It was a bug, not covered by testing. I am poor at testing my own code (I just do not use the sequence that was behind the PkK savegame. Did not cross my mind, neither while writing code nor while testing. I am grateful that this bug was captured now.

Things should work how users expect them to work.

What is the expected work? I think that casual player wants the training sites to train soldiers as well as resources permit. If soldiers are never released because that one item is missing and one step lacks, it is surprising. If site imports untrained candidates when trained ones are available, that is also surprising, to some at least.

I think I can make decent-looking icons for buttons and such if that's any help.

That is Noorfrees's decision. The timing of the freeze was a bit unfortunate, from the point of view of this modification: time for testing is a bit short. I was fully unaware of the coming freeze while doing this two weeks back.

Discussion of new buttons is a bit out of scope of this ticket, but we should think what buttons should those be? In build-20, the algorithm that chose soldiers was un-intuitive to newcomers, too. Mere "old way" and "new way" buttons are not the way to go, imho.

+1 resetting on kicking soldiers and changing capacities should be enough for now

I do my best not to provide more new bugs.

[Noordfrees]

Things should work how users expect them to work.

What is the expected work? I think that casual player wants the training sites to train soldiers as well as resources permit. If soldiers are never released because that one item is missing and one step lacks, it is surprising. If site imports untrained candidates when trained ones are available, that is also surprising, to some at least.

+1

I think I can make decent-looking icons for buttons and such if that's any help.

That is Noorfrees's decision. The timing of the freeze was a bit unfortunate, from the point of view of this modification: time for testing is a bit short. I was fully unaware of the coming freeze while doing this two weeks back.

It was declared exactly one week before the deadline: Last Wednesday evening.
Btw: last time, there was talk about going into feature freeze sometime soon but nobody stated an exact deadline, and then it was like, this approved feature will be merged now and NOW we have feature freeze and that approved bugfix will have to wait for b21. This time I've announced a deadline which will be strictly kept.
Time for testing will be enough: From now until all targeted bugs are fixed (which is a couple of weeks), and then another two weeks. Only string tweaks are forbidden, but for this bug there shouldn't be a need.

[hessenfarmer]

the release should be aimed to produce as less repeating complaints as possible.

Then we'd have to fix all 401 issues before making a release. There will always be something users complain about, and every new feature will get people saying it could be improved like this, no, like that, or…

this is a bit polemic as a lot of them are not bugs but feature requests. Anyhow I am with you that we neeed to decide not to implement new features from a specific point in time. Only thing I am concerned about is we were fully in feature developping mode and out of a sudden we are feature freezed, with perhaps some features not in a really mature state. So all I am asking for is some time to find and solve bugs. To be short feature freeze is today. Now we are in bug fixing mode. But we should take time for this and if something is turning up in the next weeks that is critical and perhaps resulting from the various changes we made we need to include this in the list of bugs for b21. We asked for feedback in the forum and we should take it serious if bugs are reported.

Let us not forget that this feature here – even if it may seem unintuitive to some users sometimes – is still always an improvement over the previous algorithm.

things that are not intuitive are to be avoided I believe. As people tend to not read manuals or even play tutorials.
One cause for different user opinions still is their usage of widelands. As importance of features and game mechanics differs between single player and multiplayer. As I am more on the single player side it is often hard to understand why things are seen different on multiplayer and vice versa. That is the only reason for playing multi from time to time to get an understanding of these issues.
Especially this feature might be seen different. on singleplayer it does not matter to have detailed control over the trainingsites. In multiplayer you really need this and time matters there.

I just wanted to clarify my position. But I am happy with tppq's solution and with the timeline given.

[tppq]

Fixed in #3871

[tppq]

Funny, now I finally see the pull request linked. The buttons to explicitly modify those are greyed out, in my view at least.

[tppq]

The current commit also addresses the request that if capacity is increased, soldiers walk in no matter what.

I put one exception there: If increasing from zero to one, the statemachine is not disabled but reset to starting position (=prefers strong soldiers). Reason being, that that is more likely a sign of active micromanagement for supersoldiers, and then getting the most trained person is probably what the player wants. Increasing to any other value disables selection.

[tppq]

put one exception there: If increasing from zero to one...

I wanted to say that aloud, since this seems to raise strong emotions which I cannot predict. Now is a good moment to yell, if that was a bad idea. Simplifying it is easy.

[spth]

Is there anything preventing us from immediately getting a best-match soldier from those that happen to be sitting in warehouses as soon as there is free capacity in the training camp?

I'd find it confusing if opening a slot would result in that slot staying empty for a while. But I'd be too lazy to complain, I'd just try clicking the slot open/close buttons a few times and see if I get any soldiers.
Personally, I expect my economy to immediately fill demand using whatever is available (even if not optimally). Empty slots in the training camp are such demand.

[Noordfrees]

The economy tries to fulfill Requests (such as empty slots in trainingsites) as soon as possible. But in order to do this most efficiently, it does this by frequently (every few seconds) iterating over all open Requests and all available Supplies and finding optimal matches. That's why immediately fulfilling demand is not possible, and we have to wait and use timeouts instead to give the economy enough time to even try to send suitable soldiers.

The current commit also addresses the request that if capacity is increased, soldiers walk in no matter what.
put one exception there: If increasing from zero to one...

+1 for both solution and exception

[tppq]

...and use timeouts instead to give the economy enough time to even try to send suitable soldiers.

The timeouts are already so short that I suppose that the next-best soldier is already now picked.

Since some of us see a problem, I am not suggesting increase into the timeouts just now.

====

UNRELATED PLEASE BE PATIENT.

One of my test games is barbarians where there are both training buildings present. After being exposed to the savegame of PkK, I also maximized the soldier capacity of trainingscamp, removed all the wares and did not stop the building. After all this, there was a huge back and forth traffic. I find this plain ugly. This is unrelated since the problem is, in my opinion, the too fast kick-out pace, and that part of the code is the same as previously. I feel the temptation to do that, but since

• Nobody does that in real game. Or does?
• There is no bug report
• Freeze is close
  thus we could also keep the code as-is. END OF UNRELATED

Apart from that one discovery, I have not found anything suspicious from the linked pull request yet.

[hessenfarmer]

Made a new bug for the unrelated part as this was a side effect of another change.
(other people (me) are trying hard to introduce bugs as well) ;-)

[tppq]

(other people (me) are trying hard to introduce bugs as well) ;-)

Writing code to deeps space gadgets must be great fun.

[tppq]

The linked pull request fixes the problem described in this ticket. I cannot promise that the fix itself would be bug free, but at this point it is unlikely that I would find bugs in it (works fine in my play style, but so did the previous, demonstrably buggy version).

[tppq]

The linked pull request fixes the problem described in this ticket.

To be more verbose: It

• fixes the bug described in bug description,
• adds the feature that increasing amount of trainees temporarily bypasses the selection,
• Modifies selection in the case that no training at all takes place, but site is active and receiving unupgradable (because of lack of stuff) soldiers.

I think that the bug really should be fixed. Even if no real harm was done in the attached savegame, in case of large Frisian training site it could have deadlocked without fix.