1. We have added a Gift Upgrades feature that allows you to gift an account upgrade to another member, just in time for the holiday season. You can see the gift option when going to the Account Upgrades screen, or on any user profile screen.
    Dismiss Notice

Frustrating bug --- please help

Discussion in 'Civ4Col - Creation & Customization' started by agnat86, Jul 22, 2013.

  1. agnat86

    agnat86 Chieftain

    Joined:
    Jun 30, 2011
    Messages:
    218
    I'm currently trying to teach myself to mod. I've just learned how to edit and compile a DLL, and I never encounter any trouble when making simple modifications.

    However, whenever I add new XML tags to XML files, things go wrong. The game crashes to desktop without a visible cause, and the only message I get is simply "Colonization has stopped working." This usually happens at the end of a turn, and it is reproducable.

    I have added a new parameter (WaterExtraYield) to Civ4TraitInfos.XML, because I wanted to make a trait for natives (Fishermen) which increases the food production on water.

    To do this, I looked at another parameter (ExtraYieldThresholds), which is the same kind of parameter (it also requires you to specify a yield and a number), searched for all occurences of this one in .cpp and .h files, and simply cloned all lines of code that contained "ExtraYieldThreshold" for "WaterExtraYield." I have based this more or less on this beginner's guide. I also made the necessary changes to CIV4CivilizationsSchema.xml.

    But while my parameter works in exactly the same way as that other one (except obviously for the functional parts in CvPlot.cpp), it causes these mysterious crashes.

    I have encountered this problem only once before, also when I tried to add an XML tag this way. Because the function of that one was completely different, it seems that this bug is caused not by an error in the code where I describe what it does, but merely by the act of adding an XML tag.

    This is really annoying, as my new parameter works exactly the way I intended it to work, and the game sometimes progresses many turns in a row without any trouble, until it suddenly crashes. Does anyone know how this happens? Is there some other place where I should define the number of XML tags that a trait could have?

    I would really appreciate your help on this, it would be hard to make new features if I were limited to the existing parameters in XML files.:sad:

    Thanks in advance.
     
  2. Kailric

    Kailric Jack of All Trades

    Joined:
    Mar 25, 2008
    Messages:
    3,094
    Location:
    Marooned, Y'isrumgone
    As far as I know there is no set number of xml tags other than physical memory. I do know you can add all the xml tags you want and it will not effect the game at all unless you modify the code the do something with it so I would first assume there is a problem in your code rather than a problem in the xml tag. If the xml is wrong the game generally will not even start up or you'll have lots of xml errors as it loads. If you followed the example of the similar attributes in the Cvinfos and all is setup correctly then the problem could be in the execution of the new attribute. Post all relevent code in the C++ files so we can have a look at it.

    Also, if you truly want to do some cool moding it is best to get a debug version up and running. You can use the free Mirosoft studio express 2008 edition if you don't have it that works with the Debug Makefile.

    Read through that thread and you will learn a bit about it. With express 2008 you can actually attach the editor to the Colonization process. So that when the games crashes or you get a "failed assert" the game will pause and you can follow then follow the code line by line in real time as you play the game. This is indispensable in bug hunting.
     
  3. raystuttgart

    raystuttgart Civ4Col Modder

    Joined:
    Jan 24, 2011
    Messages:
    4,447
    Location:
    Stuttgart, Germany
    @Kailric:
    I agree, with all you wrote. :thumbsup:
    But I suggest to choose Visual Studio 2010 C++ Express Edition.

    @agnat86:
    I you started with the "Beginner's Package" for DLL-Projects you find in my SVN, the setup for Debug-DLL is ready to go.
    (The folder is called "Workspace". There is also an explanation called "Workspace Guide". If you follow those instructions your project is set up in 1min.)
     
  4. agnat86

    agnat86 Chieftain

    Joined:
    Jun 30, 2011
    Messages:
    218
    @ Kailric & raystuttgart

    Thanks for the help! I will try to find out what goes wrong.
     
  5. Kailric

    Kailric Jack of All Trades

    Joined:
    Mar 25, 2008
    Messages:
    3,094
    Location:
    Marooned, Y'isrumgone
    Agree with 2010 but I didn't know if that makefile would work for it. Nightinggale has made a new and improved makefile just recently found here http://forums.civfanatics.com/showthread.php?p=12549883.

    I am not sure if that one works for Civ4Col but I know he made one cause we use it for the M:C project. One thing to note about the 2010 version when I first installed it it was missing some of the features like "Attach to Process". I figured out I could access it with the short cut ctrl-alt-P but soon I figured out that I needed to switch on "Expert" mode under Tools/Settings then select Expert. I have no idea why they would do that unless they truly intended you to be an expert that already knew the expert option was there. I merely found it by accident :crazyeye:
     
  6. Nightinggale

    Nightinggale Chieftain Supporter

    Joined:
    Feb 2, 2009
    Messages:
    4,116
    That's the one I use as well. Highly recommended.

    I added project files to Religion and Revolution Extended and Medieval Conquest as well. Both should compile out of the box assuming you did step 1 in the modiki. While that guide mentions a makefile, I would recommend my own, which is also part of RaRE and M:C.
    Pay special attention to "running the debugger" in that guide as this is how the computer can tell you why the game crashes. Do run in window mode when using the debugger. The debugger freezes the game when crashing and alt+tab will not work. I think control+alt+delete will allow you to move to the debugger, but that's a bit extreme for avoiding the window mode.
     
  7. agnat86

    agnat86 Chieftain

    Joined:
    Jun 30, 2011
    Messages:
    218
    I've tried running the game using a Debug-DLL, and at the point where it crashed, I got this message:

    "Assert Failed

    File: c:\users\frank\desktop\workspace\cvgamecoredll\CvPlayerAI.h
    Line: 24
    Expression: ePlayer >= 0
    Message: Player is not assigned a valid value"

    I do not understand what this means, I'm afraid.

    This is the piece of code I added to CvPlot.cpp, containing three new variables that allow traits to increase yields on land, water and hills tiles respectively (my additions in bold:

    Spoiler :


    iYield += GC.getGameINLINE().getPlotExtraYield(m_iX, m_iY, eYield);

    if (ePlayer != NO_PLAYER)
    {
    if (GET_PLAYER(ePlayer).getExtraYieldThreshold(eYield) > 0)
    {
    if (iYield >= GET_PLAYER(ePlayer).getExtraYieldThreshold(eYield))
    {
    iYield += GC.getDefineINT("EXTRA_YIELD");
    }
    }
    }

    if (!isWater())
    {
    if (!isImpassable())
    {
    CvCity* pWorkingCity = getWorkingCity();
    if (NULL != pWorkingCity)
    {
    if (!bDisplay || pWorkingCity->isRevealed(eTeam, false))
    {
    iYield += GET_PLAYER(ePlayer).getLandExtraYield(eYield);
    }
    }
    }
    }

    if (isWater())
    {
    if (!isImpassable())
    {
    CvCity* pWorkingCity = getWorkingCity();
    if (NULL != pWorkingCity)
    {
    if (!bDisplay || pWorkingCity->isRevealed(eTeam, false))
    {
    iYield += GET_PLAYER(ePlayer).getWaterExtraYield(eYield);
    }
    }
    }
    }

    if (isHills())
    {
    if (!isImpassable())
    {
    CvCity* pWorkingCity = getWorkingCity();
    if (NULL != pWorkingCity)
    {
    if (!bDisplay || pWorkingCity->isRevealed(eTeam, false))
    {
    iYield += GET_PLAYER(ePlayer).getHillsExtraYield(eYield);
    }
    }
    }
    }



    and a second part:

    Spoiler :

    int iExtraYieldThreshold = 0;
    for (int iTrait = 0; iTrait < GC.getNumTraitInfos(); iTrait++)
    {
    CvTraitInfo& trait = GC.getTraitInfo((TraitTypes)iTrait);
    iExtraYieldThreshold = std::max(trait.getExtraYieldThreshold(eYield), iExtraYieldThreshold);
    }
    if (iExtraYieldThreshold > 0 && iMaxYield > iExtraYieldThreshold)
    {
    iMaxYield += GC.getDefineINT("EXTRA_YIELD");
    }

    if (!isWater() && !isImpassable())
    {
    int iLandExtraYield = 0;
    for (int iTrait = 0; iTrait < GC.getNumTraitInfos(); iTrait++)
    {
    CvTraitInfo& trait = GC.getTraitInfo((TraitTypes)iTrait);
    iLandExtraYield = std::max(trait.getLandExtraYield(eYield), iLandExtraYield);
    }
    iMaxYield += iLandExtraYield;
    }

    if (isWater() && !isImpassable())
    {
    int iWaterExtraYield = 0;
    for (int iTrait = 0; iTrait < GC.getNumTraitInfos(); iTrait++)
    {
    CvTraitInfo& trait = GC.getTraitInfo((TraitTypes)iTrait);
    iWaterExtraYield = std::max(trait.getWaterExtraYield(eYield), iWaterExtraYield);
    }
    iMaxYield += iWaterExtraYield;
    }

    if (isHills() && !isImpassable())
    {
    int iHillsExtraYield = 0;
    for (int iTrait = 0; iTrait < GC.getNumTraitInfos(); iTrait++)
    {
    CvTraitInfo& trait = GC.getTraitInfo((TraitTypes)iTrait);
    iHillsExtraYield = std::max(trait.getHillsExtraYield(eYield), iHillsExtraYield);
    }
    iMaxYield += iHillsExtraYield;
    }

    return iMaxYield;
    }

     
  8. raystuttgart

    raystuttgart Civ4Col Modder

    Joined:
    Jan 24, 2011
    Messages:
    4,447
    Location:
    Stuttgart, Germany
    You are missing security checks. ;)

    e.g. checks like this:

    if (ePlayer != NO_PLAYER)
     
  9. Nightinggale

    Nightinggale Chieftain Supporter

    Joined:
    Feb 2, 2009
    Messages:
    4,116
    It mean a function is called with player as an argument where it is supposed to be a real player and instead it's NO_PLAYER (which is -1)

    The original code does
    Code:
    if (ePlayer != NO_PLAYER)
    	{
    		if (GET_PLAYER(ePlayer)
    You addition uses GET_PLAYER() without checking if ePlayer is different from NO_PLAYER. Change your code to only execute if ePlayer != NO_PLAYER and it should work. Well it should fix THIS bug.
     
  10. agnat86

    agnat86 Chieftain

    Joined:
    Jun 30, 2011
    Messages:
    218
    Great, I've added three security checks to the first part, and now it works fine. Thanks very much!:goodjob:
     
  11. Kailric

    Kailric Jack of All Trades

    Joined:
    Mar 25, 2008
    Messages:
    3,094
    Location:
    Marooned, Y'isrumgone
    Cool, I hope what we have here is an awesome up and coming Colonization modder!! :cool:

    I had all kinds of questions like these when I first started. Fans of the game on these forums are all about helping, well as long as you don't have to drive anywhere :crazyeye:.

    Your problem was and still is one of my major pit falls. If its a variable in the function that means there is a chance that it will be NULL or -1 so always remember to do security checks.

    I have a question too while we are here. Why in the code is there a getOwner() and a getOwnerINLINE() ?

    When all getOwner() does is...

    Code:
    PlayerTypes CvCity::getOwner() const
    {
    	return getOwnerINLINE();
    }
    I think I messed up several times by doing a GET_PLAYER(getOwner()) as doing a GET_PLAYER(getOwnerINLINE()) would actually skip a step and would seem to be faster.
     
  12. raystuttgart

    raystuttgart Civ4Col Modder

    Joined:
    Jan 24, 2011
    Messages:
    4,447
    Location:
    Stuttgart, Germany
    getOwnerINLINE() is simply an inline function (thus supposed to be faster) that is available at certain objects (Unit, City, Plot, ...).

    There are other inline functions as well, e.g. getGameINLINE().

    I have tried to read and understand tutorials about inlined functions a while ago.
    It is not necessarily that clear.

    It is a non-standard construct that some compilers support and interpret to optimize the generated code.
    It is supposed to eliminate time overhead of the call.

    Actually it is still not that clear to me, when to explizitly inline functions or not.
    Since when you do not explicitly tell the compiler, the compiler might try to optimize (and inline) by itself.

    However, when inline functions already exist, I always call these instead of non-inline functions if those are available as well.

    Edit:

    If you have a wrapping function around an inline function, it is usually only done to give less experience programmers a method with type checking.
    (Since inline functions do not have type checking and thus it is more likely to make errors when using them.)

    That is probably the answer to your question why both methods exist.
     
  13. Nightinggale

    Nightinggale Chieftain Supporter

    Joined:
    Feb 2, 2009
    Messages:
    4,116
    It's a very tricky issue, but there is one case where it is always beneficial.

    Code:
    #ifdef _USRDLL
    	inline PlayerTypes getOwnerINLINE() const
    	{
    		return m_eOwner;
    	}
    #endif
    Return a variable is always faster when it is inline and the dll will be smaller as that inlined return statement will generate less code than a function call.

    You should NOT use inline if the function produces too much code. The problem is that the CPU cache is limited and the bigger the dll, the more likely it is that the next part of the code isn't in the cache and the CPU will have to wait for the RAM.

    While we are at the mysteries: #define _USRDLL looks odd as it is always defined. As it is defined in the makefile VC++ couldn't figure it out until I modified the project file to tell which flags the makefile defines (hence a reason to download the project files I uploaded). Red lines for undefined flags in compilable code isn't a good thing. In fact it was rather annoying.
    I think it's a "leftover" from when the game was compiled and never removed just to annoy modders.


    It would be a bit faster, but I don't think you will be able to tell the difference. It's not the place where the big speed boost is hidden.

    There is one potential problem with the code you wrote. Do you know for sure that getOwnerINLINE() will not return NO_PLAYER?
     

Share This Page