Bug Reports

[Nightinggale]

This is going to be tricky as I pulled the fix, but it wasn't updated in the DLL when I pulled so when I tested it there was still that bug. So, even though it said all was up to date I had to rebuild the dll and then commit/push the fixed dll.

We'll have to remember to pull all three each time and to push all three each time.

Maybe the reason why you had to compile is that I intentionally didn't commit the DLL. I don't like the to commit a DLL alone because if we end up with several hundred MB of DLL files, we can delete them. Nobody really needs the old ones anyway and they can always be recompiled. The issue with this approach is that if somebody adds DLL files and nothing else in those commits, then we can risk having bogus commits with no file changes.

 

[Kailric]

I've had this Fail Assert pop a couple times now. I attached a saved game. This game was saved while play testing my latest push to source, the Upkeep Anarchy push which I am finishing pushing it now. It should fail the next turn. I haven't look into this yet.

File: CvUnit.cpp
Line: 12808
Expression: getFeatureDoubleMoveCount(eIndex) >= 0
Message:

 

[Nightinggale]

getFeatureDoubleMoveCount(eIndex) >= 0

That is quite strange. It can happen like this:

Unit gain a promotion giving double movement in terrain eIndex. The counter increase by 1.
Unit loses the promotion and the counter decrease by 1.
Unit loses the same promotion again and the counter decrease by 1 and ends up on -1.

I have no idea how this can happen. Did you do something, which give negative values to double movement in terrain? From the look of it, it is only affected by promotions and is recalculated on load. It looks fine and is fairly easy to read as it is only changed from line 13280 in CvUnit:rocessPromotion(). It isn't like we can question where in the code it is changed.


Speaking of quite strange bugs, I encountered one in CvXMLLoadUtility::SetFeatureStruct(). Col2071 crashes on startup when it tries to allocate memory for aaiFeatureYield, which is a vector of vectors. Not only does this code looks just fine, it also appears to be unchanged from vanilla
I tried going back in revisions to avoid whatever memory related I touched recently, but that didn't help. Now I will rewrite it into an array of JIT arrays just like profession yield costs. If nothing else, it moves the memory allocation code from the vector into our code and I will see which line it fails in. Also just like with profession costs, it will not allocate arrays unless it contain non-zero data. This will save some memory and maybe we can save some array looping somewhere.

The error I get in output. The debugger isn't very helpful as it happens inside the exe.

Heap corruption detected at 0563B008
Heap corruption detected at 0563B0F8
First-chance exception at 0x7c910efe in Colonization.exe: 0xC0000005: Access violation reading location 0x00000000.
Unhandled exception at 0x7c910efe in Colonization.exe: 0xC0000005: Access violation reading location 0x00000000.

EDIT problem solved and the solution is pushed to the source. I still don't get why the code crashed in the first place though.

 

[Kailric]

I haven't done anything with promotions for a while. I did some work during the Civics Branch a while back. I made it so that "Civilian Promotions" are never lost or taken away. I did this because they would lose the promotion when joining a city. I don't know if that has anything to do with it or not. There maybe a better solution for that.

Edit: Actually, I can't remember exactly how I fixed this. I'll look it over too when I get the chance.

 

[Nightinggale]

I did some work during the Civics Branch a while back. I made it so that "Civilian Promotions" are never lost or taken away. I did this because they would lose the promotion when joining a city. I don't know if that has anything to do with it or not. There maybe a better solution for that.

Analyzing the assert with the debugger tells that is triggered by UNIT_NATIVE joining a city.

It has the real promotion PROMOTION_RANGER2. This mean FeatureDoubleMoves should go down for forest and light forest, which appears to be correct. The bug is that the cached value in the unit is 0 when it appears like it should have been 1. Next question is why isn't it 1?

 

[Nightinggale]

Fixed the bug. The promotion cache has a feature, which released unused memory. A typo/copy paste error made the code skip the unused part and the promotion effect reverted to 0. As this is no longer, the fix will not affect savegame compatibility and games are fixed on load.

 

[Nightinggale]

I just had a crash. I used a civ with land start, bought a pirate ship without discovering the eastern access plots, then I had an assert with NULL starting plot followed by a crash.

 

[Lib.Spi't]

yeah that happened to me in the past with not having the spice route discovered.

 

[Kailric]

I just had a crash. I used a civ with land start, bought a pirate ship without discovering the eastern access plots, then I had an assert with NULL starting plot followed by a crash.

I redid the code for this but I remember it got "stashed" and never commited. I'll fix this asap.

 

[ethernidee]

http://wikisend.com/download/273454/AutoSave_AD-0794.ColonizationSave

Next turn we face the following bug: any attempt to save game produces infinite size file. Em, 272 MB and we kill the process. Any attempt to end turn (after the next turn) produces stable crash. We played stable mod version and that was the first tiny map we tried. Quite disappointing the results is. Anyway, I post here s saved game for you guys to check when you decide to fix some older bugs.

 

[Nightinggale]

http://wikisend.com/download/273454/AutoSave_AD-0794.ColonizationSave

Savegames can be attached to the forum as well.

Next turn we face the following bug: any attempt to save game produces infinite size file. Em, 272 MB and we kill the process. Any attempt to end turn (after the next turn) produces stable crash. We played stable mod version and that was the first tiny map we tried. Quite disappointing the results is. Anyway, I post here s saved game for you guys to check when you decide to fix some older bugs.

I never heard anything like that before for any mod

I really wonder how this could happen. 272 MB is 71 million ints. The only thing I can think of is saving an array where the array length is corrupted. This could explain both file size and crash. Imagine if it is the counter for number of units. It loops the now 100 million units for a player (or whatever it is) and crashes when it reaches the first unallocated one.

Next is the question of how that can happen. I have no idea whatsoever. Considering nobody else experienced this issue, it could be something as uncommon as a flipped bit in memory.

We really should make a release soon because

1. poor communication failed to tag the last release in git
2. bug reports for last release are somewhat out of date

I don't know which revision the savegame was made with and and I have done major reworks to the savegame format. That makes this really hard to figure out. On the other hand if we have a bug, which causes this and it is still in our code, then it is really important that we find it.

 

[ethernidee]

I downloaded stable version from the main page 2.0h. We repeated our actions several time. Crash is constant. Colonization version is 1.0.0.1.
Probably nobody played on tiny map against 12-13 AI in Hot Seat earlier. Should I replace dll with Assert DLL and check again?

 

[Lib.Spi't]

isn't 2.0i the latest version?

I also have a feeling that Col. would need patching too....

 

[ethernidee]

Right 2.0i, 2.0h is readme.
I also changed <bIsNativeTrade> to 0 for Salt and Spices and added BARBARIAN_PEDLER profession:

{
"ProfessionInfo": {
"Type": "PROFESSION_BARBARIAN_PEDLER",
"Description": "Native Peddler",
"Civilopedia": "???",
"Strategy": "???",
"bNativesInvalid": "0",
"bEuropeInvalid": "1",
"bColonialInvalid": "1",
"Combat": "NONECOMBAT_PEDLER",
"DefaultUnitAI": "UNITAI_TRADER",
"SpecialBuilding": "NONE",
"UpgradeProfession": "PROFESSION_MERCHANT",
"bWorkPlot": "0",
"bCitizen": "0",
"bWater": "0",
"bScout": "0",
"bCityDefender": "0",
"bCanFound": "1",
"bUnarmed": "0",
"bNoDefensiveBonus": "0",
"iCombatChange": "2",
"iMovesChange": "0",
"iWorkRate": "0",
"iMissionaryRate": "0",
"iPower": "0",
"iAsset": "0",
"FreePromotions": {
"FreePromotion": {
"PromotionType": "PROMOTION_PEDLER",
"bFreePromotion": "1"
}
},
"YieldsProduced": { "YieldType": "NONE" },
"YieldsConsumed": { "YieldType": "NONE" },
"Button": ",Art/Interface/Buttons/Civilizations/Colonization_Civilizations_Leaders.dds,6,7"
}
}

Hard to say what is screwed.

 

[Nightinggale]

For me to load your savegame I need the file(s) you modified because 2.0i is extremely picky about using the very same XMl files. It isn't enough for me to just add PROFESSION_BARBARIAN_PEDLER. It also needs to be in the same order. When I try to load it, I get number of cities for player 0 (the top one) to be 0, because there is an offset error of 1 due to having an additional profession.

The main reason for doing major rework on the savegame format since 2.0i is that array lengths and indexes are now stored in savegames. This mean the game can figure out what each int is without asking the XML files and that kind of offset errors should be a thing of the past.

On top of that I must add that I haven't tried hotseat games. I think they should work just fine. However we know that network games are broken. I have a plan for fixing that, but it isn't the top priority.

 

[ethernidee]

You're right, sorry.
Attached the necessary files.

 

[Nightinggale]

The savegame is corrupted. The first city for the first player has quite a lot of yields set to be exported. More precisely the savegame claims the number to be 0x40000. The array to store those in has a length of 0x22 meaning fun stuff happens when the game loads and tries to write to this array.

Cause: unknown, but it happened when or before saving meaning we can't catch the bug in the act. I mentioned the risk of a flipped bit and 0x40000 is indeed a single bit away from the presumably correct 0.

Luckily I can say that the code in question is not only modified in git, it is completely removed and replaced with standard JIT array saving code. I don't think it will be worth the time to investigate this any further unless it reappears with the new savegame format.

 

[Kailric]

I found my stashed fixed for the Purchase Sea Units in Trade Screens not appearing Bug and pasted/pushed them into the current Source. So, hopefully that is fixed.

 

[ethernidee]

Nightinggale, thank you.

 

[Kailric]

I introduced a bug one of my last pushes were you no longer gain Trading Points after the first Trade Tech. I was attempting to remove all the old Vanilla Constitution code but removed to much

I have a problem with the new Trade Screen code. When I award players the Spice Route for moving on to the tile the award is lost after a saved game. When the game first initializes all trade routes are set to True. Then if it requires a tech and the player doesn't have it it is set to false. Now, when I award the player the Trade Route all is fine until the game is reloaded then it is set to false again in updateInventionEffectCache().

Technically I am thinking that initial trade routes only need to be set up once when the game initializes. The variable m_ba_TradeRouteTypes is saved so there is no need to reconfigure it on load up. I only added it there for the initial changes. So, we should be able to move this initial set up to another part of the code. Would that work best in CvPlayer::init or CvPlayer::reset or some where else, or should something else be done?

Edit: now that I am more awake I see that this should be done in the init function as this is only run once to setup the player I assume.

I could just award the trade tech Spice Route as well but then that messes up the Trade Point screen as Currency is then researchable when technically it should not be until all other techs before it are researched.