worker::find_object calls productionsite::unnotify too often

Bug #1437818 reported by daAlx1
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Won't Fix
Undecided
Unassigned

Bug Description

While reviewing the code of the function
bool Worker::run_findobject I found something that in my
opinion may be unintended behaviour/bug, but I am not sure about it,
so I would like to ask an experienced wdelands developer to comment on this:

1. run_findobject can be used to find immovables or bops.
As far as I understand, the worker can communicate to the player
via the functions notify_player and unnotify_player of its
production site in order to raise a warning if he cannot find an
object to do his work.

In case the worker searches an immovable, then the code guarantees
that one of the following happens
- the worker finds a good object
- there is no object of the desired type in the search radius of the
worker, and the function calls notify_player
- there is an object in search range, but all of such objects are currently
tagged by workers. Then the function calls unnotify_player

When searching a bop, the notification mechanism is called in a
completely different way: For each search radius from 1 to the maximal
search distance of the worker, it calls unnotify_player if there is
no good object in this search radius, and finally, if it did not
succeed to find an object at all it calls notify_player.

So:

 If a hunter searches for an animal and there is one at distance 5,
he calls 5 times unnotify_player and then finds the object.

If there is no animal close enough, he will call several times (his
search radius, to be precise) unnotify_player and then call once
notify_player

If this is the intended use of notify and unnotify, I ask someone
to explain the logic behind this. Otherwise I
propose to use in both cases the notification style that is now applied
when searching immovables. By this one could significanlty simplify the
code as well (especially once my fix to Bug 1407418 has been merged).

Revision history for this message
TiborB (tiborb95) wrote :

daAlx1,

I was looking at this code just last week but I was not interested in notifying stuff so I dont know answers, but any simplification and especially speeding up the code is highly welcomed.

Maybe if you start tinkering with code you will find out why it was needed :)

Revision history for this message
GunChleoc (gunchleoc) wrote :
Changed in widelands:
status: New → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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