First release

We need to get SimCutie's event manager released first. It would seem to me that his stuff contains many of the big features that make this mod a good base for others to work off (options screen & event manager).

After that's done, then we can start doing stuff for the next big merge (see the 2nd release thread), which may warrent a new thread - I don't think we really need a new one yet.

 

Yep, start up python error problem cleared. and Two new improvement.

1. Now DLL no longer require any Assets/XML or python file to installed in players assets folder to operate properly. If no Assets exist it will remain dormant. No "you have defeated". (Version in CVS dev branch)

2. DLL now can read old firaxis game save file.
But one problem. It clashes with Impaler's More Form of Commerce features.
It can be fixed but requires quite a work before we can have both features.
We should modify every read ( NUM_COMMERCE_TYES, ....) to be modified slightly. quiet a work. I have to disable More form of Commerce features disabled temporarily before modify them all. (working version)

 

We're not going to be able to maintain complete save-game compatablily with Firaxis saves for ever. Wasn't the plan just to make it so that it doesn't cause them to crash, but instead raises an error?

Will it be able to be complete for a release later today/tomorrow?

 

I've looked at the Code and I see you also deactivated the localized Specialist Commerce and Yield Changes. I am again quite peved at your misplaced priorities and disregard for others. Do you seriously think backward savegame compatability is so important that it warents throwing out two major new feature? Secondly why didn't you say something 2 days ago when you made the CVS comitment or *gasp* actualy ask other people what they thought of it. I'm not going to claim my code is infalible or that it must be included but not even asking is just plain rude.

Secondly please refrain from going through and trying to neaten/butify other peoples code to your personal standards. I see you have wraped comments around code that was already perfectly well commented with mod/name/date and someone (I'm not shure if this is intentional or being done unwitingly by you texteditor) is striping off trailing whitespace from hundreds of lines. Finding a bug or vulnerability is fine but totaly non-substantive changes make for endless pages of diffs on WinMerge greatly complicating code merger

I would ask you hold up for a day long so I can get my latest mod it, I need to resolve a conflict on several files and see that the merger compiles well, I hope to have it sometime late tonight if all goes well. I would also ask that the savegame conflict be discussed and we can come to a consensus on what is more important. I think you all know ware I stand on the issue.

 

I agree that removing your stuff isn't right - I hadn't actually checked the files but these will certainly not be released with at least the functionality of the old files. I had presumed that they would go back in once he had tweaked them so that they were save-game compatable.

I think I'll go and bump that thread on save-game compatablitly.

 

You mis-read source in CVS. The version in CVS has all Impaler's features intact including More form of commerce. I haven't touched features.
And I have no intension to throw away any features of Impaler's code except one: "RogerBacon's AirComatMod" (mutiple targetting per turn for air unit)
It is currently active without any switch or XML tag. It changes games behavior without players acknowlegement. So it is currently disabled until we have add switch to enable it with XML (like <MultipleTargetting>3</MultipleTargetting> ) or anything.

I haven't touched localized Specialist Commerce and Yield Changes featrues in CVS version and even in working version. Only More form of commerce is affected currently in working version (Not in CVS version)and it is temporary measure to debug. I said "I have to disable More form of Commerce features disabled temporarily before modify them all. (working version)" It will be fully restored in finished version.

Some of your code (especailly from Rogerbacons's codes) have serious ambigulity on where the change ends. I have to see original code to know where the mod ends. I added BEGIN/END comment in such case.
And As I pointed out some of your comment hides original line.

Code:

 Ex) 
ORIGINAL CODE:
        a = 1;
YOUR COMMENT:
        // Original a = 1;
        a = 2;
CHANGED:
       /*// Original
      a = 1;
      */
      a = 2

Keeping original line intact is important for using diff. tools.
It is not my arbitray personal taste. It is universal manner of any developer who work in a team.
Your code is not your personal property once it is commited to team project. It should be public shared property.

About stripping white space : I also object to stripping trailing white space in source.
My rationale : Keep original source line as much as it was before as possible.
I am not sure that it was my fault or not. But if it turns out my fault. I will try to restore it back to original as much as possible.

 

The incressed AirUnit combat dosn't require any control because all existing air units were given 1 movment by Firaxis in addition to the code setting their movment to zero after a single mission. The player would need to change the XML values to see any effect.

 

Movement value should not used for that.
It is generally used as distance of land unit movement, not number of times for attacking enemy units per turn.
Think of the case like some other moder uses "AirRange" XML attribute of land units (they are all zero for all land unit) to set number of times land unit can attack multiple enemy per turn. Is it proper usage of "AirRange" attribute?
Multiple targetting of air uit should have explicit XML attribute for that purpose.

 

It is a perhaps a bit sloppy, but it keeps the schema and file neat...

 

I dont consider it bad because its logical, Firaxais already makes use of flexible interpritation of the the Movment value based on the Domain, Air units dont "move" (just place one with WorldBuilder and you will see) they have "missions" instead. Its logical for them to get more missions with more movment points as these concepts are equivilent when applied to the Air Unit. RB made good use of an existing data member and XML atribute that were basicaly being wasted. The fact that no change occures untill values are moded means it conforms to our standards for inclusion.

Using AirRange for any purpose on a Domain Land unit would be confusing because it would lack a clear meaning like the AirUnit had. The destinction is significant so I must disagree with you on RB's mod whial agreeing with your example being a bad behavior.

On the Commerce Mod, what is the ETA on finishing the backward savegame compatability and reactivating these features?

Also I fail to see how /* */ commenting is superior to a // when your dealing with oneline changes (Big blocks ofcorse its better). When you diff the first you end up with 2 seperate line addition blocks rather then one alteration block, this would seem to me to be much more straitforward and easier to get at with WinMerge as it produces fewer diff blocks to scroll through and the chance of missing one is reduced. We all agree original code presevation is important and this is achived. Everything beyond that is personal taste, you yourself continue to ifdef everything despite being told by almost every other member of the project that it makes the code hard to read and is useless for a project of this nature.

 

Is there any other case when move value is used for denote targetting number? None. Other (flexible) interpretaion of move in other domain always related to diatance not number of times.
Your interpretation of move value as targetting number is arbitrary choice not logical choice. How about interpreting move value of air unit as "re-base" diatance range limit per turn? It seems more logical to me than number of attack.

I agree. It will confuse ther moder. Your interpretaion of move valus is confusing, too. move should be always interpreted as distance (number of plots/grid) not number of times. they has different unit.

Current version in CVS does not have backward savegame compatability and but the version is Commerce Mod enabled.
I will not upload to CVS version with backward savegame compatability only version (but the with Commerce Mod disable.)
To have both features enabled, it is not so simple job, it may take few days.

With your way of //Original ... original line..., orginal lines are hided and buried in modified block. I should intentionally parse the modified block to know what part is original. and what part is modified code.
If we keep original line as unmodified by line itself, WinMerge will automatically diffrentiates original part and mdified /added /deleted part visually. That is the purpose we use the tool, isn't it?

If #ifdef was such a useless tools that only degrades code readablility, why is it used so widly in other commercail/open-source project? Have you ever did regression test of your code without #ifdef?
It is valueable tool that makes code maintenance and testing much easier than not using it at all.

 

Right, so we'll update when commerce thingy and backward compatabilty are both working, ok?

 

I've uploaded my latest UU 4 leaders mod along with some Bug fixes and a Dll based Debugging feature (a simple print text to screen feature) from 12Monkeys that I had forgoten to merge in to the first release (I put it on the Extentions.cpp)

I fixed quatation mark on CvExtentions that was causing a compiler error it now reads
GC.setDefineSTRING("EXTDLL_BUILD, EXTDLL_BUILDER , EXTDLL_VERSION", __TIMESTAMP__ );

and removed a extra </TEXT> tag in one of SimCuties text.xml's that was causing a load error. Game compiled and ran well for me.

Am now experimenting with Dales Bombardment mod, trying to convert it to a method which dose not remove original style fortification reduction. I will most likly break it into 2 buttons one for "Reduction" (eqivilent to old Bombardment) and one for "Bombardment" (new ability active only if unit has new XML tag with none zero value.

 

This is not what I have intended. "EXTDLL_BUILD" is setDefineSTING () first argument, variable key(name). But EXTDLL_BUILDER , EXTDLL_VERSION is macro symbol defined as string constant.
Define EXTDLL_BUILDER , EXTDLL_VERSION as compile time symbol definition in compiler/preprocess option.
In compiler cbp file, EXTDLL_BUILDER=""Impaler[WrG]"", EXTDLL_VERSION=""161.1.012"".
You may / may not need two consecutive double-quotation, because one(1) double- quotation mark will be eat up by Windows DOS command shell.
This should be equivalent to #define EXTDLL_BUILDER "Impaler[WrG]" in C++ source code text.

Then EXTDLL_BUILDER "," EXTDLL_VERSION "," __TIMESTAMP__ is 5 sting constant (no comma between all string constants) will merged as single C++ sttring constant like "Impaler[WrG],161.1.012,Wed Jun 21 13:22:52 2006"
and used as second argument to GC.setDefineSTRING().
This is used as DLL build identification. and will be print in log file like line:
"DLL Build: SimCutie,161.1.016,Wed Jun 21 13:22:52 2006".

 

That wont compile the Funtion sees it as an excessive number of arguments
Perhaps this is what you wanted?

GC.setDefineSTRING(" 'EXTDLL_BUILD','EXTDLL_BUILDER','EXTDLL_VERSION' ", __TIMESTAMP__ );

Do you want us to change thouse Defines each time we compile a Binary for Commiting to the CVS?

PS - I am still getting the crash at clicking Play on Dawn of Man screen, am I correct in thinking the fix
for this is not uploaded yet but you do have a fix ready?

 

No. this should be equivalent like..

Code:

#define EXTDLL_BUILDER        "Impaler[WrG]"
#define EXTDLL_VERSION       "161.1.xxx"

GC.setDefineSTRING("EXTDLL_BUILD", EXTDLL_BUILDER "," EXTDLL_VERSION "," __TIMESTAMP__ );

first arg is "EXTDLL_BUILD"
Second arg is EXTDLL_BUILDER "," EXTDLL_VERSION "," __TIMESTAMP__
It is single sting, not 3 argument. Two "," is string of a comma, not argument delimeter.

Accessing undefined GC.getDefineSTRING("<undefine>" ) may cause crash.

 

Can I submit a last minute entry?

I have added these two functions to the end of CvUnit.cpp. What they do is self explanitory.

Code:

CvUnit.cpp

//Returns the number of units carried by this unit
int CvUnit::getNumberUnitsCarry( const CvPlot* pPlot ) const
{
    int iNumUnitsPlot;
    int iNumUnitsCarry;
    int iI;
    CvUnit* pLoopUnit;

    iNumUnitsPlot = pPlot->getNumUnits();
    iNumUnitsCarry = 0;

    for (iI = 0; iI < iNumUnitsPlot; iI++)
    {
        pLoopUnit = pPlot->getUnit(iI);

        if (pLoopUnit->getTransportUnit() == this)
        {
            iNumUnitsCarry++;
        }
    }

    return iNumUnitsCarry;
}

//Returns the unit occupying a slot by this unit.
//Slots are 0 indexed
CvUnit* CvUnit::getUnitCarry( const CvPlot* pPlot, int iSlot ) const
{
    int iNumUnitsPlot;
    int iNumUnitsCarry;
    int iI;
    CvUnit* pLoopUnit;

    iNumUnitsPlot = pPlot->getNumUnits();
    iNumUnitsCarry = 0;

    for (iI = 0; iI < iNumUnitsPlot; iI++)
    {
        pLoopUnit = pPlot->getUnit(iI);
        if( (pLoopUnit->getTransportUnit() == this ) )
        {
            if( iNumUnitsCarry == iSlot )
            {
                return pLoopUnit;
            }
            iNumUnitsCarry++;
        }
    }
    return false;
}

Code:

CvUnit.h


public:

	int getNumberUnitsCarry( const CvPlot* pPlot ) const;
	CvUnit* getUnitCarry( const CvPlot* pPlot, int iSlot ) const;

 

getNumberUnitsCarry function seems to duplicated functionarity with CvUnit::getCargo() which returns number of cargo unit the transport Unit is currently carrying. What is difference with CvUnit::getCargo() ?
What is meaning of pPlot argument? Is it same as CvUnit.getPlot()?
!2Monkey's code has similar function that loops through all cargo unit of the transport unit. I will try to merge your code with 12Mokeys code.

 

OK. I added a function that returns list of cargo unit for given transport unit.

CLinkList<IDInfo> *CvUnit::getCargoList();
Retuens NULL if no unit is carried on the unit.

CLinkList<IDInfo> *pCargoList = pUnit->getCargoList();

number of cargo units = pCargoList->getLength();
get first cargo -->
CvUnit *pUnit = ::getUnit(pCargoList->head());
loop through all cargo Unit of pTransportUnit-->

Code:

        CLinkList<IDInfo> * pCargoList = pTransportUnit->getCargoList();
        CLLNode<IDInfo>* pNode = pCargoList-> head(); 
         CvUnit *pLoopUnit ; 
       while ( pNode != NULL && ( pLoopUnit  = ::getUnit(pNode)) != NULL )
      {
                pLoopUnit -> ............ // do some thing on pLoopUnit
                pNode  = pCargoList-> next(pNode ); // step to next in list...
       }

How about that?

 

That wont compile, its like I said too many arguments, I think

GC.setDefineSTRING(EXTDLL_BUILD," EXTDLL_BUILDER, EXTDLL_VERSION, __TIMESTAMP__ ");

But now I see EXTDLL_BUILD is undefined, did you compile the code that was originaly on the CVS because I had the original problem with the code strait off the CVS without any modifications. We should always make shure that code being commited will at the very least compile.

Lord Olleus: If these new functions are to be used by Python Moders they need to be exposed to the Python layer which is relativly simple to do but just takes a bit more work. Was the intent of the second function to iterate through all the units in a Plot which are cargo (regardless of how many different transports they are in) whial skiping the transports themselves? Or is it for iterating through the Cargo of a single transport vessel. In the former the function should be called from CvPlot, in the latter case I agree the Plot argument is unessay it can be found internaly. Also you cant return false from a function that has a Pointer return type, you want to return NULL.