New Version - November 13th (11/13)

Status
Not open for further replies.
What's the rationale behind this change?

Also, are all these changes part of the CP as well? Because the change with DoW's requiring an embassy seems to go beyond bugfixing and into what CBP is trying to do.

It's a CBP-only feature (the DOW embassy and City-DOF thing) for now, while testing. If successful, it'll be put in CP, as it is an AI performance and quality measure.

G
 
I agree, what is the rationale behind this change? It makes sense that my past enemies would want their cities back after a bad war by giving me a lump sum of gold, but they have to be my friends also to buy their cities back? :confused:
I agree, buying back your own city should ideally be exempt from this rule. However according to the other thread, you can trade cities as part of a peace negotiation while (obviously) not being friends, but I guess that might be difficult to afford for the losing party.
 

Attachments

  • dump+trace.zip
    28.9 KB · Views: 48
I agree, buying back your own city should ideally be exempt from this rule. However according to the other thread, you can trade cities as part of a peace negotiation while (obviously) not being friends, but I guess that might be difficult to afford for the losing party.

Hostile parties cannot and should not buy and sell land from each other. If you want land and you are not their friend, your recourse is war. This feels far more natural, and I thank Edaka for pointing out what should have been blatantly obvious to me.

G
 
New test version inbound

Here's the link: Mega Beta Folder

Bittersweet, nice to see you incorporated my suggestions into the code, horrible to see they did not make a difference...

Crash at turn 14.

The drudgery continues...

Only thing I can think of now, is that if you are getting index-out-of-bounds exception here:

Code:
UnitHandle pUnit = m_pPlayer->getUnit(*it);

... then perhaps it is a good idea to constrain the loop to values only under the end of the iterator (as it is now, any value that is not the final of the iterator will continue the loop, even if the value is out of bounds of getUnit(it)...), like this:

Code:
for(it = m_CurrentTurnUnits.begin(); it < m_CurrentTurnUnits.end(); it++)

I know it generally works, but if it is there that you are getting IOOB exceptions, then obviously somehow it > CurrentTurnsUnits.end() in some cases... why not eliminate that possibility altogether?

Just shooting blindly again, but maybe, just maybe...

Either that, or CurrentTurnUnits somehow has more unit IDs than whatever list the function getUnit(int) calls from...

Another possibility is that having the same name for the iterators (it) in nested loops may confuse the compiler? They are smarter now, but AFAIK it is a big NONO in programming to use same names for nested variables, especially in loops... I know, scope and all that, but maybe this is what is loading the variable "it" more than it should (combined with the soft limit in the for loop, != instead of <), and then you get the OOB exception... what if you use slightly different variable names for the internal loops, like it2, it3, and so on...?
 
Bittersweet, nice to see you incorporated my suggestions into the code, horrible to see they did not make a difference...

Crash at turn 14.

The drudgery continues...

I didn't think those changes would make a difference, but it was worth trying while ilteroi and I continue apace.

What's your choice of map and civ, if I may ask? Do these choices make a difference for the CTD for you? Do you restart the map once loaded, or just dive straight in? Are you using the standard civ lua, or are you using ilteroi's moded lua51 file?

Also, if you have a tacticalAI.log file, post it please.

Thanks,
G
 
Hostile parties cannot and should not buy and sell land from each other. If you want land and you are not their friend, your recourse is war. This feels far more natural, and I thank Edaka for pointing out what should have been blatantly obvious to me.

G
Well I consider it more like a hostage exchange. It's not a major deal for me, but I do think it's very likely that there will be situations where you might not find a swap right at the time of peace negotiations acceptable because one part is likely to be starved for resources at that time - whereas a trade at a later time could be meaningful. That's why I think that ideally (and by ideally I mean: I don't know if it's possible to code this) but ideally, a player should always be able to make a trade offer for a city they've founded themselves, whereas trades that involve giving away one of their own cities (and possibly also third party cities) only should be possible between DoF friends.
 
What's your choice of map and civ, if I may ask? Do these choices make a difference for the CTD for you? Do you restart the map once loaded, or just dive straight in? Are you using the standard civ lua, or are you using ilteroi's moded lua51 file?

Fractal standard, does not make a difference as the initial crashes happened on Continents, no restart, standard LUA.
 
Bittersweet, nice to see you incorporated my suggestions into the code, horrible to see they did not make a difference...

Crash at turn 14.

The drudgery continues...

Only thing I can think of now, is that if you are getting index-out-of-bounds exception here:

Code:
UnitHandle pUnit = m_pPlayer->getUnit(*it);

... then perhaps it is a good idea to constrain the loop to values only under the end of the iterator (as it is now, any value that is not the final of the iterator will continue the loop, even if the value is out of bounds of getUnit(it)...), like this:

Code:
for(it = m_CurrentTurnUnits.begin(); it < m_CurrentTurnUnits.end(); it++)

I know it generally works, but if it is there that you are getting IOOB exceptions, then obviously somehow it > CurrentTurnsUnits.end() in some cases... why not eliminate that possibility altogether?

Just shooting blindly again, but maybe, just maybe...

Either that, or CurrentTurnUnits somehow has more unit IDs than whatever list the function getUnit(int) calls from...

Another possibility is that having the same name for the iterators (it) in nested loops may confuse the compiler? They are smarter now, but AFAIK it is a big NONO in programming to use same names for nested variables, especially in loops... I know, scope and all that, but maybe this is what is loading the variable "it" more than it should (combined with the soft limit in the for loop, != instead of <), and then you get the OOB exception... what if you use slightly different variable names for the internal loops, like it2, it3, and so on...?

Worth a shot. Since the secondary (nested)

I've been scouring the code for initialization errors, which were my initial (ha) assumption here. The fact that I simply cannot reproduce the CTD on my and (nor can ilteroi, for what it is worth) is what makes this so hard to debug. Everyone's minidumps are breaking in the same location (right on that getunit element), which gives you one of two possibilities:

The m_pPlayer call is breaking (nigh impossible, as the function itself is called by m_pPlayer)

OR

The iterator is going OOB or calling a bad pointer.

Since the former is nigh impossible, the second is the most viable area of interest. The other problem here, though, is that the same currentmoveunit loop is called over and over again in the cvtacticalAI, which makes me think that there's something broken in the ranged recruitment model for safebombards that's seeding bad data into that function. But, then again, that's doesn't necessarily make sense either, as there are other ranged unit calls that aren't breaking.

This is the loop that I'm in right now.

G
 
I've try a quick test and the previous save still crash at the same point (turn 38), but in a new game no crash still (turn 70).

Shortly after resuming newly CTD. I've tried last Ilteroi's dll and standard/modified lua51_Win32.dll without difference.
 
I've used the new Beta builds and ilteroi's last installment.

This time it crashed at turn 10/11.

Attaching minidump and tracespy's last words...

Also may be worth noting that I couldn't see the city bar (the one on which you would usually click to access the citizen management etc.). Also the citizen management didn't seem to work at all.

EDIT: Playing on communitas map, large, standard speed; no tech brokering, no vassalage.
 

Attachments

  • anothercrash.zip
    28 KB · Views: 37
Updated, played multiple games past turn 200, still no crashes here. Let me know if you want more info.

Btw, I think the DoF change for trading cities is excellent.
 
I'm also testing with [fractal, standard, quick]. No luaJIT and no other mods to the base files (except some loose texture overrides I'm working on). No restarts. I've used the tuner to autoplay some sessions, but the result is the same without it. Likewise for different map or speed settings, though quick speed does produce crashes faster.

Ran with ilteroi's dll from post#138 alongside tracespy... it crashed within a dozen turns or so. This was with clean cache and loading only the Community Patch (from Gazebo's post#123) and the replacement dll.

Trace Spy output and minidump are attached
 

Attachments

  • hh_dump+trace_01.7z
    235.3 KB · Views: 37
There is an option that I proposed in the beginning and that may benefit, well, EVERYONE. It makes sense to only use so much time to try and solve this issue; after such amount is used, it becomes a drag. Further fixes and improvements to the whole CBP cannot be done with full focus, while at the same time a portion of the users cannot use the newest versions.

I think we reached such time. I think the best to do now, for ALL of us, is to revert the offending code to its state of 5-11. Granted, it wasn't the optimized ranged units behavior, but it was stable. In the meanwhile, Ilteroi can try to continue to track the nasty bug in the newest code, with the help of some of us, but in the sidelines and not as part of the main CBP development line.

I can live with a sub-optimal range unit behavior, if that is the price to keep using the latest versions of the whole mod. I also think others can, as the price for giving us the "crashers" the chance to continue to be part of this mod, and also for releasing G's focus from this issue so that he can continue the meaty work on the CBP as a whole, which would benefit ALL of us.

That is what I propose.
 
...
The iterator is going OOB or calling a bad pointer.

Since the former is nigh impossible, the second is the most viable area of interest. The other problem here, though, is that the same currentmoveunit loop is called over and over again in the cvtacticalAI, which makes me think that there's something broken in the ranged recruitment model for safebombards that's seeding bad data into that function. But, then again, that's doesn't necessarily make sense either, as there are other ranged unit calls that aren't breaking.

This is the loop that I'm in right now.

G

Looking at this from the other end... why would that bomb on some systems and not others? Maybe those of us experiencing this crash should compare notes on installed redistributables/runtimes, physical ram, OS version etc. vs those who are running smoothly.

I went on a bit of a witch hunt recently to solve a problem I was having with Visual Studio (broken toolchain). It's fixed (so I think :rolleyes:), but in the process I cleaned up the .net stack and a big pile of runtimes, reinstalling the latter as needed. Scrutinizing that now...
 
hello again.

i think i got it. anyone care to give it a try? i still don't know why it was crashing only for some people, but there's a spot where it could plausibly crash (iterator invalidated by a remove call on an stl container).

https://dl.dropboxusercontent.com/u/16988401/civ/CvGameCore_Expansion2.zip

Looking good so far :goodjob:

Running autoplay and going out for a bit, but it's already well past where it would have crashed... ranged units are in play and doing their thing.

Only thing of note, Trace Spy is showing a pile of these statements...

GetPlotsAtRangeX() called with invalid parameter
 
Status
Not open for further replies.
Top Bottom