(attempted, failed) Asynchronous damage calculation

Brainstorm ideas of possible additions to the game. Read this before posting!

Moderator: Forum Moderators

Forum rules
Before posting a new idea, you must read the following:
Post Reply
jyrkive
Inactive Developer
Posts: 36
Joined: July 7th, 2016, 6:20 pm
Location: Helsinki, Finland

(attempted, failed) Asynchronous damage calculation

Post by jyrkive »

I usually only send messages in #wesnoth-dev, but I have so much to say at once that I decided to make a forum post instead. @mods, feel free to move this topic if there is a more appropriate subforum.

Zookeeper suggested in IRC on July 17 that the game, when the widescreen theme is used, could calculate predicted battle results asynchronously (in a background thread) in order to show them in the sidebars. Currently the game calculates them synchronously, and can freeze for seconds at worst if the upcoming battle is complex (very high HP, high number of strikes, drain ability, etc.)
20160717 15:54:15< zookeeper> and if it does the calculation on mouseover to allow showing the results as some kind of tool-tip like thing, it'd be nice if _that_ was 1) not done unless the results are actually shown and 2) threaded so it doesn't cause a delay anyway. sadly, nice doesn't equal easy.
20160717 15:54:58< JyrkiVesterinen> I can add that to my todo list.
I tried that, but it turned out that it's too hard to do after all. I decided to give up.

The problem comes down to race conditions. It isn't safe to access game state in a background thread without external synchronization because the main thread may be manipulating the game state at the same time. That may result in corruption and crashes. Worst of all, race conditions are timing related and very hard to fix.

The sidebar code operates by generating so-called reports. This function generates the report about the weapons of an unit, which is the report that requires damage calculation.

Among other things, it calls a function called attack_info():

Code: Select all

base_damage = attack_info(rc, *context_unit_stats.weapon, res, *u, unit_loc);
Attack_info() is a long function. It accesses game state, it e.g. loops over every unit in the map:

Code: Select all

for (const unit &enemy : rc.units())
It also augments the report on its own:

Code: Select all

add_text(res, flush(str), flush(tooltip));
There are two ways to make attack_info() safe:
  1. Only ever call it in the main thread (this is what Wesnoth currently does).
  2. Protect everything that attack_info() can access with a mutex.
The second approach would be too hard, I'm afraid. It would require adding locking code all over the place.

So, I tried the first approach. I split the calling function into two parts, where the first part runs in the main thread and calls attack_info(), and the second part runs in a worker thread and performs the slow damage calculation: https://github.com/jyrkive/wesnoth/blob ... s.cpp#L870

It's safe from race conditions... but it unfortunately reorders stuff in sidebars.

Every call of attack_info() adds information to the report. The existing code makes the calls intertwined with a loop that goes through the attacks, whereas mine does all the attack_info() calls first and only then adds the predicted outcomes to the report. Result:
Sidebar.png
Sidebar.png (32.77 KiB) Viewed 1391 times
It shows first both ways in which the Bone Shooter can defend itself (the dagger and the bow), and then the expected HP distributions for both. It's just broken.

Now, this would be fixable with even more refactoring... but at this point I decided that this is becoming just too much effort for too little gain, and decided to throw in the towel. I am not going to implement asynchronous damage calculation.

If anyone wants to see the work-in-progress implementation I made, it's here: https://github.com/jyrkive/wesnoth/comm ... alculation

----

I think the best way forward here is to just speed up damage calculation in complex cases.

(Hmm, a post this long looks like a rant. Rest assured that I'm not ranting: it's just my writing style to be thorough. :) )
Post Reply