MantisBT - VCMI
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0001949VCMIMechanics - Town structurespublic2014-11-13 19:132014-12-22 09:46
ReporterSXX 
Assigned ToSXX 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformPCOSLinux KubuntuOS Version14.04
Product Version 
Target VersionFixed in Version0.97b 
Summary0001949: Rampart's Treasury doesn't increase gold income
DescriptionIt's should give 10% increase per town, but at moment it's not work for me on any map.

At moment trying to investigate it on my own a bit. :-)
Steps To Reproduce  - Start game with "Rampart" on any map that allow select town.
  - In next days build "Dwarf Cottage", "Miners' guild" and "Rampart's Treasury".
  - Now just wait to next week and check if you get extra income on first day.
Additional InformationI'm still running version from "ppa:vcmi/ppa".
Though it "vcmi_0.97+git20141112.3778~ubuntu14.04.1" now.

All mods except "VCMI essential files" are disabled.
TagsNo tags attached.
Relationships
Attached Files? BUG_TREASURY_DAY_7.vcgm1 (2,021,621) 2014-11-13 19:13
https://bugs.vcmi.eu/file_download.php?file_id=1960&type=bug
? BUG_TREASURY_DAY_7.vsgm1 (2,022,248) 2014-11-13 19:13
https://bugs.vcmi.eu/file_download.php?file_id=1961&type=bug
txt server_log.txt (4,893) 2014-11-13 19:14
https://bugs.vcmi.eu/file_download.php?file_id=1962&type=bug
txt VCMI_Client_log.txt (1,495,505) 2014-11-13 19:14
https://bugs.vcmi.eu/file_download.php?file_id=1963&type=bug
txt VCMI_Launcher_log.txt (5,378) 2014-11-13 19:14
https://bugs.vcmi.eu/file_download.php?file_id=1964&type=bug
txt VCMI_Server_log.txt (937,482) 2014-11-13 19:14
https://bugs.vcmi.eu/file_download.php?file_id=1965&type=bug
Issue History
Date ModifiedUsernameFieldChange
2014-11-13 19:13SXXNew Issue
2014-11-13 19:13SXXFile Added: BUG_TREASURY_DAY_7.vcgm1
2014-11-13 19:13SXXFile Added: BUG_TREASURY_DAY_7.vsgm1
2014-11-13 19:14SXXFile Added: server_log.txt
2014-11-13 19:14SXXFile Added: VCMI_Client_log.txt
2014-11-13 19:14SXXFile Added: VCMI_Launcher_log.txt
2014-11-13 19:14SXXFile Added: VCMI_Server_log.txt
2014-11-13 20:18SXXNote Added: 0005090
2014-11-14 14:56SXXNote Added: 0005100
2014-11-14 15:19SXXNote Added: 0005101
2014-11-14 15:19SXXNote Edited: 0005101bug_revision_view_page.php?bugnote_id=5101#r2686
2014-11-14 23:15SXXNote Added: 0005104
2014-11-15 22:02SXXNote Added: 0005105
2014-11-15 22:15SXXNote Edited: 0005105bug_revision_view_page.php?bugnote_id=5105#r2688
2014-11-16 01:56SXXNote Added: 0005106
2014-11-16 01:56SXXNote Edited: 0005106bug_revision_view_page.php?bugnote_id=5106#r2690
2014-11-16 07:19WarmongerNote Added: 0005107
2014-11-16 07:56SXXNote Added: 0005108
2014-11-16 07:57SXXNote Edited: 0005108bug_revision_view_page.php?bugnote_id=5108#r2692
2014-11-16 08:29SXXNote Edited: 0005108bug_revision_view_page.php?bugnote_id=5108#r2693
2014-11-16 08:29SXXNote Edited: 0005108bug_revision_view_page.php?bugnote_id=5108#r2694
2014-11-16 22:23SXXNote Edited: 0005108bug_revision_view_page.php?bugnote_id=5108#r2695
2014-11-16 22:59SXXNote Added: 0005112
2014-11-16 22:59SXXNote Edited: 0005112bug_revision_view_page.php?bugnote_id=5112#r2697
2014-11-16 23:28SXXNote Added: 0005113
2014-11-16 23:34SXXNote Edited: 0005113bug_revision_view_page.php?bugnote_id=5113#r2699
2014-11-17 00:37SXXNote Added: 0005114
2014-11-21 12:12SXXNote Added: 0005142
2014-11-21 13:42WarmongerNote Added: 0005143
2014-11-22 19:07SXXNote Added: 0005150
2014-11-22 19:07SXXStatusnew => resolved
2014-11-22 19:07SXXFixed in Version => 0.97b
2014-11-22 19:07SXXResolutionopen => fixed
2014-11-22 19:07SXXAssigned To => SXX
2014-12-22 09:46SXXStatusresolved => closed

Notes
(0005090)
SXX   
2014-11-13 20:18   
So as I find out in "CGameHandler::NewTurn" in /server/CGameHandler.cpp this check return false all the time:
t->hasBuilt(BuildingID::TREASURY, ETownType::RAMPART)
(0005100)
SXX   
2014-11-14 14:56   
As I find out set "builtBuildings" doesn't contain ID of treasury for some reason. Now trying to find out why is that happening. :-)
(0005101)
SXX   
2014-11-14 15:19   
From what I see "BuildingID::TREASURY" matches int 23, but for treasury itself written into "builtBuildings" matches int 22. So likely contents of builtBuildings isn't actually correct at least for treasury.
(0005104)
SXX   
2014-11-14 23:15   
Just checked that "NewStructures::applyGs" in "/lib/NetPacksLib.cpp" also getting int 22 when I build treasury.
(0005105)
SXX   
2014-11-15 22:02   
(edited on: 2014-11-15 22:15)
So in logs of VCMI there is line that stated following:
core : building.rampart.special3 -> 22

So it's looks like Treasury have to be named as "special3" which is same thing as int 22.

In same time in "/lib/GameConstants.h:337" BuildingID::TREASURY set as SPECIAL_4:
		TREASURY            = SPECIAL_4,


I'm not yet sure if there anything else depend on this code and why "Fountain of Fortune" using SPECIAL_3, but I'll try to find out.
(0005106)
SXX   
2014-11-16 01:56   
At moment in-game buildings looks this way:
Mystic pond == 17 == building.rampart.special1
Fountain of Fortune == 21 == building.rampart.special2
Rampart's Treasury == 22 == building.rampart.special3

In same time current code handle it this way:
MYSTIC_POND == 17 == SPECIAL_1
FOUNTAIN_OF_FORTUNE == 22 == SPECIAL_3
TREASURY == 23 == SPECIAL_4

Anyway It's looks like those IDs hardcoded for reason and for me it's looks like there is some conversion between building ID's in H3 assets and VCMI so from this point some advice from someone who know codebase better.
(0005107)
Warmonger   
2014-11-16 07:19   
I guess you could just fix these in config/factions config files.
(0005108)
SXX   
2014-11-16 07:56   
(edited on: 2014-11-16 22:23)
Of course I have no idea how VCMI works, but all other castles using following "special" according to GameConstants.h:330

Castle: 1,2,3
Tower: 1,2,3,4
Inferno: 2,3,4
Necropolis: 1,2,3
Dungeon: 1,2,3,4
Stronghold: 1,2,3,4
Fortress: 1,2,3
Conflux: 1,2

Why is Rampart have 1,3,4 here?
    MYSTIC_POND         = SPECIAL_1,
    FOUNTAIN_OF_FORTUNE = SPECIAL_3, //Rampart
    TREASURY            = SPECIAL_4,
(0005112)
SXX   
2014-11-16 22:59   
So I find out commit that added this code into "GameConstants.h":
https://github.com/vcmi/vcmi/commit/0ca9f64573b68d0cca1bbc3860a107ba805d0b58#diff-74ba6bc82c538f5dc1f3c6ba2e99a56c [^]
After it's was initially added it's never changed.

There also interesting change including with this commit that change "CGameHandler.cpp" line that handle how Treasury works:
 			if(!firstTurn)
-				if (t->subID == 1  && player < GameConstants::PLAYER_LIMIT && vstd::contains(t->builtBuildings, 
EBuilding::SPECIAL_3))//dwarven treasury
+				if (t->hasBuilt(EBuilding::TREASURY, ETownType::RAMPART) && player < GameConstants::PLAYER_LIMIT)
                         
n.res[player][Res::GOLD] += hadGold[player]/10; //give 10% of starting gold



So it's all looks like there was typo in code and special's "1,3,4" have to be changed to "1,2,3" in "GameConstants.h".
(0005113)
SXX   
2014-11-16 23:28   
(edited on: 2014-11-16 23:34)
So as I find out this small error also break "Fountain of Fortune" because it's bonus assigned to SPECIAL_3 (ID 22, Treasury). :-)
Of course I tested it too.
(0005114)
SXX   
2014-11-17 00:37   
I tested this change and it's looks like working just fine and fixed both problems.

No idea if this right thing to do or not, but sent pull request:
https://github.com/vcmi/vcmi/pull/56 [^]
(0005142)
SXX   
2014-11-21 12:12   
This one now can be closed as resolved. :-)
(0005143)
Warmonger   
2014-11-21 13:42   
Now you can do that on your own ;)
(0005150)
SXX   
2014-11-22 19:07   
Thanks. Hopefully I'll able to help VCMI project more in future.