0002373: Necromancy skill rises troops incorrectly

(0006325)
Kantor (updater)
2016-01-20 18:52
edited on: 2016-02-14 15:44

After some digging into the code, here is my proposition to fix this issue:
replace 997 line in lib/mapObjects/CGHeroInstance.cpp with this one:
if ((necromancyLevel > 0 || hasBonusOfType(Bonus::IMPROVED_NECROMANCY)) && !battleResult.casualties[!battleResult.winner].empty())

(0006421)
SXX (administrator)
2016-02-13 18:37

Just in case this one is easy fix, but before it's applied we'll actually need to check other Necromancy behaviour. It's complex skill and I suspect there may be more bugs around.

Any chance you already tested necromancy more than just for that bug?

(0006422)
Kantor (updater)
2016-02-13 19:00
edited on: 2016-02-13 19:03

Unfortunately not, as of now I'm unable to compile VCMI by myself. I'm pretty sure that necromancy works properly for other cases.
Right now necromancy skill will always rise at least 1 troop, even if both sides were untouched after battle.

(0006423)
SXX (administrator)
2016-02-13 21:09

Ok there is map for advanced necromancy skill testing. :-)

(0006428)
Kantor (updater)
2016-02-16 20:23

I just checked this issue with my solution, and that solves it completely.
Next thing to do is to check if necromancy rises correct amount of troops, but I doubt there are any issues with this.

(0006432)
SXX (administrator)
2016-02-17 08:18

Your solution does solve it, but I would rather check math inside instead.
Also I don't think calculation is correct, but testing would take a while.

(0006434)
Kantor (updater)
2016-02-17 14:36

Hmm, there indeed is a issue with calculation for hero which rises upgraded creatures and has artefact like Cloak of Undead King.
From 100 Obsidian Gargoyles hero (with adv. necromancy) in OH3 would rise 30 wraiths, in VCMI that would be 29.

(0008568)
Ivan (developer)
2023-04-12 17:06

Migrated to github, status will be tracked here:
https://github.com/vcmi/vcmi/issues/1219 [^]

(0008913)
Ivan (developer)
2024-01-30 19:18

Closing issues that were migrated to Github

Issue History
Date Modified	Username	Field	Change
2016-01-13 20:29	Kantor	New Issue
2016-01-13 20:29	Kantor	File Added: necromancy test.h3m
2016-01-20 18:52	Kantor	Note Added: 0006325
2016-02-13 18:37	SXX	Note Added: 0006421
2016-02-13 19:00	Kantor	Note Added: 0006422
2016-02-13 19:03	Kantor	Note Edited: 0006422	View Revisions
2016-02-13 21:09	SXX	File Added: SXX_Necromancy_v1.h3m
2016-02-13 21:09	SXX	Note Added: 0006423
2016-02-14 15:44	Kantor	Note Edited: 0006325	View Revisions
2016-02-16 20:23	Kantor	Note Added: 0006428
2016-02-16 20:26	Warmonger	Assigned To	=> Warmonger
2016-02-16 20:26	Warmonger	Status	new => assigned
2016-02-17 08:18	SXX	Note Added: 0006432
2016-02-17 14:36	Kantor	Note Added: 0006434
2023-04-12 17:06	Ivan	Status	assigned => confirmed
2023-04-12 17:06	Ivan	Note Added: 0008568
2024-01-30 19:18	Ivan	Note Added: 0008913
2024-01-30 19:18	Ivan	Status	confirmed => closed

Notes
(0006325) Kantor (updater) 2016-01-20 18:52 edited on: 2016-02-14 15:44	After some digging into the code, here is my proposition to fix this issue: replace 997 line in lib/mapObjects/CGHeroInstance.cpp with this one: if ((necromancyLevel > 0 \|\| hasBonusOfType(Bonus::IMPROVED_NECROMANCY)) && !battleResult.casualties[!battleResult.winner].empty())

(0006421) SXX (administrator) 2016-02-13 18:37	Just in case this one is easy fix, but before it's applied we'll actually need to check other Necromancy behaviour. It's complex skill and I suspect there may be more bugs around. Any chance you already tested necromancy more than just for that bug?

(0006422) Kantor (updater) 2016-02-13 19:00 edited on: 2016-02-13 19:03	Unfortunately not, as of now I'm unable to compile VCMI by myself. I'm pretty sure that necromancy works properly for other cases. Right now necromancy skill will always rise at least 1 troop, even if both sides were untouched after battle.

(0006423) SXX (administrator) 2016-02-13 21:09	Ok there is map for advanced necromancy skill testing. :-)

(0006428) Kantor (updater) 2016-02-16 20:23	I just checked this issue with my solution, and that solves it completely. Next thing to do is to check if necromancy rises correct amount of troops, but I doubt there are any issues with this.

(0006432) SXX (administrator) 2016-02-17 08:18	Your solution does solve it, but I would rather check math inside instead. Also I don't think calculation is correct, but testing would take a while.

(0006434) Kantor (updater) 2016-02-17 14:36	Hmm, there indeed is a issue with calculation for hero which rises upgraded creatures and has artefact like Cloak of Undead King. From 100 Obsidian Gargoyles hero (with adv. necromancy) in OH3 would rise 30 wraiths, in VCMI that would be 29.

(0008568) Ivan (developer) 2023-04-12 17:06	Migrated to github, status will be tracked here: https://github.com/vcmi/vcmi/issues/1219 [^]

(0008913) Ivan (developer) 2024-01-30 19:18	Closing issues that were migrated to Github

Relationships