Multiple hunters hunt for same animal

[giffel]

When one animal enters operating area of multiple hunters, all of them will start chasing it, but only the fastest hunter will succeed and others will return without prey. Unfortunately, this behavior occurs even when more than one animal is in sight. It decreasing productivity of hunters and some assignment of animal to one hunter might be beneficial (same situation as tree is assigned to exactly one lumberjack).

[Hans Joachim Desserud (hjd)]

Hi giffel, thanks for reporting this issue.

Marking confirmed based on your screen shot. If we indeed have such an assignment of trees to a specific lumberjack when they set out to do their work (I haven't checked the code), then we should probably have similar systems in place for other workers.

[SirVer (sirver)]

IIRC the trees are tagged by the workers. I am not sure if tags are only in place for immovables or also for bobs.

[daAlx1 (amdat)]

I have had a look at the source code in order to find a solution to this bug.

Currently, only immovables have functions is_reserved_by_worker() and set_reserved_by_worker(bool reserve) which set / read a flag whether the immovable is target of some worker and worker::run_findobject() uses these to tag the found immovable (if it is used to find an immovable).

In order to solve this bug, animals must have a similar mechanism. Currently, a hunter calls run_findobject in search for an object of type bob which has attribute eatable.

So a simple approach to solve this bug could be the following:

Move the is_reserved_by-worker and set_reserved_by_worker from the class Immovable to its parent MapObject and tag everything which is found using run_findobjects(). (As a byproduct this would simplify and shorten the code)

I have one questions to people with a good overview of the code: Is it desireable that every program running find object tags its object? Or are there situations where we want to have two workers choosing the same object. I could imagine that defending soldiers could use run_findobjects() to find attacking enemy soldiers. In this case all defending soldiers should attack a single soldier that approaches the territory so tagging would not be desired here. I have not yet figured out how fighting works, so I am not sure whether my thoughts have any contact with reality.

So base question is: Are there situations where it is desirable that several workers choose the same MapObject.

If this is the case, the solution to the hunting problem may be to reimplement is_reserved_by_worker() and set_reserved_by_worker(bool reserve) in the class Bop and to use them only if we are searching for eatable bops.

I have seen that there is also a class critter. Do I understand right that hunters actually only hunt critters? Then possibly one could rewrite their program to search for eatable critters instead of eatable bops and implement the tagging only for critters.

Anyway, in the end a solution would add some tagging mechanism to some class that does not have one yet. Hence any solution I could think of would introduce another bool to some class (MapObject, Bop or Critter) . So if I understand right, this bool would have to be saved and loaded using the class' save and load method. So it would cause incompatabilities with older saved games. Is it a problem to change the format of save games? What is the policy on downwards compatability?

[GunChleoc (gunchleoc)]

Welcome to Widelands and thank you for taking on this bug :)

In order to answer some of your questions:

- Soldiers don't use "findobject", so shifting things to MapObject should be fine. Less code is always good :)

- Hunters only hunt critters

- We will remove all savegame compatibility with the next version, so breaking savegames is OK, as long as the maps will load. If your changes are affecting the loading/saving of "world" objects, you will need to add some compatibility code.

[SirVer (sirver)]

Just to inject: Having a regression test for this would be nice too and could give you an easy way to test. Just add one animal and two hunter houses to the map and see that one hunter does not leave his building.

> Is it desirable that every program running find object tags its object?

I think that is the case. The only instance I can think of where it is useful to have two workers work the same object is for ship construction sites - and there it is not much harm if one worker only start working as soon as the other one is done.

I agree that moving this to map object makes the most sense. There is already a tagging system for map objects in place - for example "tree" is a tag that defines trees. Extending this to be dynamic and work for "runtime" tags (i.e. on instances) is a nice solution IMHO.

Great that you want to work on this!

[daAlx1 (amdat)]

Thank you for your answers. By now I have done a minimal set of changes in order to solve this bug (and I will soon learn how to make by branch available to you for possible merging with trunk).

1. I agree with SirVer that a regression test would be nice, but I am not completely sure how to do this. I will clearly manage to create a map with two hunters and one animal such that only one hunter should leave the building. But once I have this map, how do I implement a check that the expected behaviour actually happens?

2. While thinking about possible draw-backs of tagging all MapObjects a new question popped up in my mind. Imagine the following situation: We have a map with two players and both have one lumberjack each. Imagine there is only one tree on the border between the two territories such that the lumberjack of each player could reach this tree. In the current implementation if player one's lumberjack tags the tree then player two's lumberjack cannot find a untagged tree and will report that he is out of resources and stop his work. But if his hut is closer to the tree than the one of player one's lumberjack it would be very sensible to choose this tree, walk there faster then the other players lumberjack and cut the tree before the other lumberjack arrives.

This example brings me to the idea that the tagging mechanism should be player dependent: For each player, any object should only be choosable by at most one worker of this player, but the tagging should not interfere with the tagging for opponent players. Is my reasoning correct? Would it be a good feature to make the tagging player dependent? (Implementing this could be just one part of SirVer's idea of having a dynamic runtime tagging system.)

3. During my inspection of Worker::run_findobject, I saw quite some other things that do not appear to be very logical. But since it is not related to this concrete bug, I feel that I should discuss this things at some other place. Should I create a bug-report about these things? Where is the right place to discuss whether the current function implements the desired behaviour or only something close to the desired behaviour?

[SirVer (sirver)]

1. This is a bit clunky, but you run such a test using regression_test.py from the base directory (you will probably need to use the -b parameter). The individual test maps live in test/maps/. One (complex) example is test/maps/expedition.wmf which is a map you can just load up in the editor.

this map will get launched through the python script using widelands --scenario=<map>, this runs thetest/maps/expedition.wmf/scripting/init.lua script. The regression_test.py script also makes sure that after the game is running, one other script of the ones test/maps/expedition.wmf/scripting/test*.lua runs. A test runs succeeds it the string "All Tests passed" is in stdout.txt.

2. Player dependency sounds over complicating things. Overengineered imho, go for the simplest thing that works. If it turns out it is not good enough for the practicality, we iterate. Code complexity is an important factor to consider too. tl;dr: your initial idea is IMHO much better.

3. A bug is always a good starting point.

[GunChleoc (gunchleoc)]

We have a Wiki page on how to create a branch: https://wl.widelands.org/wiki/BzrPrimer/

[daAlx1 (amdat)]

Thank you, GunChleoc, for pointing me to this page. I already had uploaded a branch (lp:~amdat/widelands/HunterBug1407418) some days ago and then I got unexpectedly cut off from the internet for some days.

Could you have a look at my branch and tell me whether it is ready for merging with trunk?

Concerning regression tests: I had a look at these test scripts but I have not yet figured out how to implement the test I need. In particular, I have no idea how to check what (and where) some hunter is currently doing. I would be very thankful if someone else could provide this regression test.

[GunChleoc (gunchleoc)]

Best visit your branch and click on "Propose for merging". This way, we will get a diff :)

[daAlx1 (amdat)]

I tried to propose my fix (https://code.launchpad.net/~amdat/widelands/HunterBug1407418) for merging. Have I done it correctly? How long does it usually take until a proposed bugfix is merged?

[GunChleoc (gunchleoc)]

This can vary - it depends on when another developer finds the time to review, and if any changes need to be made to the code.

At the moment, only SirVer and I do code reviews for C++, and I'm not very experienced. So, it can take some time :(

[SirVer (sirver)]

> only SirVer and I do code reviews for C++

this is not by design though. Please feel free to start doing code reviews too - of course you cannot review your own code, but if you start reviewing somebody else's, this frees up me or Gun to look at your work :).

[GunChleoc (gunchleoc)]

Fixed in build19-rc1.