First release

I downloaded alpha version from sourceforge. 1-61-0001aSC.zip
and compiled it.
Thing to fix:
the ZIP file is missing two files: FVariableSystem.inl, CvGameCoreDLL.vcproj
So I added it and compile in Release configuration.
It compiled without problem. Run it. Selected Single Player/Play Now! (newgame). It showed Dawn of Man screen. But it freeze or in endless loop I can hear music/narration of Dawn of Man. But the screen freezed at 1/5 of(" Initilizing...) progress bar.
When loading old saved game, It immedately exited to desktop without any warning or messsage.

Part of "PythonDebug.log"
Code:
load_module Oasis

load_module Ring

load_module Shuffle

load_module Team_Battleground

load_module Terra

load_module Tilted_Axis

load_module Wheel

PY:Player 0's alive status set to: 1
PY:Player 1's alive status set to: 1
PY:Player 2's alive status set to: 1
PY:Player 18's alive status set to: 1
shifting hints by 
 
0
 
0

load_module CvGameInterface

player
 
0
 
pass
 
0
 
failed

player
 
0
 
pass
 
1
 
failed

player
 
0
 
pass
 
2
 
failed

player
 
0
 
pass
..............
This "player /failed / pass" thing loops infinitely.
 
What I think we are going to have to do is have each new mod added one at a time untill we find out which one is faulty. Unfortunately this could take a while. This is why we really need to have the CSV in use right away so code can be added and tested as it is created and not 2000 lines at once.
 
My guess on this matter:
- This code that prints this infinite loop is neither in our code, nor SDK code.
It is in Civ4.exe, I guess. This is caused by "load_module CvGameInterface" or some code that requires "CvGameInterface".
Which comes much earlier than normal timing of appearance of this message in "PythonDebug.log"

Normal loading time should be like this.:
Code:
PY:Player 1's alive status set to: 1
PY:Player 2's alive status set to: 1
PY:Player 18's alive status set to: 1
shifting hints by 
 
0
 
0

99 SCREEN TURNED ON

SCREEN OFF

load_module encodings.latin_1

8500 - gold

8501 - research
.............
8538 - taoism

3 SCREEN TURNED ON

[B]load_module CvGameInterface[/B]

99 SCREEN TURNED ON

PyCivSim: Init Done.
load_module CvWBInterface

load_module CvWBDesc
....

PS) I build SDK with my code removed from source. ( by effectively undefine all SIMCUTIE_XXX ).
But the sympthom seems just as same as before with my code compiled in.
But there is a possiblity that I may have forget to enclose my code with #ifdef ( I found one or two while reviewing in new SF's merged code ), and that part may cause the fault.

Here I attached source with all my code disabled. (Only updated part)
 

Attachments

The Great Apple said:
@Everybody - Any other opinions on this [commenting style]? It's definatly not to late to change over to a more distinctive style like SimCutie suggests, and he certainly makes some good points. Good that we have these discussions earlier rather than later. We really need to all be doing the same thing.
Any more thoughts?
 
The Great Apple said:
Any more thoughts?


I think we should do it the Firaxis way, because its the way the poeple know and which they expect. If they look for an include then they will look at the top of the file and not somewhere else. Also the additional work we had with this style when merging our changes into another update, can be ignored compared to the overall work.

Also, I think its much more important to prevent #ifdefs to only where it is absolutly necessary, because there is a big disadvantage. If the #ifdef section of a certain mod, starts to overlap with the #ifdef of another mod, its getting extremly complitcated to make the code work for both comibnations. The resulting code may be hard to read and hard to test.

12m
 
I'm a bit confused where I can find what version.

The version on SourceForge does not seem to contain any changes (at least not mine). Also the changed Python Files are missing. But some version numbers are changed to 1.3?

The CvGameCoreDLL_v161 Version on the FTP Server can not be downloaded (for whatever reason)

What is the actual version, what should I DL for testing? Anything I missed?

12m
 
12monkeys said:
I think we should do it the Firaxis way, because its the way the poeple know and which they expect. If they look for an include then they will look at the top of the file and not somewhere else. Also the additional work we had with this style when merging our changes into another update, can be ignored compared to the overall work.

Also, I think its much more important to prevent #ifdefs to only where it is absolutly necessary, because there is a big disadvantage. If the #ifdef section of a certain mod, starts to overlap with the #ifdef of another mod, its getting extremly complitcated to make the code work for both comibnations. The resulting code may be hard to read and hard to test.

12m
OK. I will change #inclide position to Top of the file as Firaxis's way.
Instead, I suggest use of # ifdef for all our code be adpoted as general guide line.
Resolving overlapping #ifdef is exactly the work we should have to do.
The #ifdef section is not random peice of code. It is logical unit of change for a feature. So if same potion of code should be changed diffrently by combination of feature, it should be logically resolved depending on combination of feature. Not using #ifdef is hiding problem under carpet, not solving it. That is not right thing to do.
Current problem of merged version (by SF) is also same problem. Each piece of code worked without problem before merging. But when they are combined, it caused problem. So it is our task to solve it logically. This is what "integration" is for. if all code used #ifdef for each individual feature, the integration test will be much eaiser, not harder.
Most major open-source software developed by multiple developer adopts such convention (i.e. compile time feature selection by #ifdef adopted by apache web server)
Some uses automatic compile timme configuration generation with autoconf.
Why shouldn't we?
12monkeys said:
I'm a bit confused where I can find what version.

The version on SourceForge does not seem to contain any changes (at least not mine). Also the changed Python Files are missing. But some version numbers are changed to 1.3?

The CvGameCoreDLL_v161 Version on the FTP Server can not be downloaded (for whatever reason)

What is the actual version, what should I DL for testing? Anything I missed?

12m
See post of mine ( http://forums.civfanatics.com/showpost.php?p=4121760&postcount=29). Use "dev" branch to get latest version in CVS.
 
The Great Apple said:
Any more thoughts?
As minimum, original line should be remained intact line by line. Commenting out original line by preceeding // will hinder comparison of source file with diff tools.
And please, don't use comment at end of line to notify start of change.
It is visually hard to find.
Code:
Bad example:
...
// Original     a =  1;
          a = 2;         // start of change for foo.
....
Code:
Good example:
....
/* Original 
        a = 1;
*/
       // start of change for foo
        a = 2;

On commnet style on individual function.
How about to use Firaxis function commneting style, then.

From original Firaxis function. CvUnit.cpp
Code:
//------------------------------------------------------------------------------------------------
// FUNCTION:    CvUnit::computeWaveSize
//! \brief      Computes the wave size for the round.
//! \param      bRangedRound true if the round is a ranged round
//! \param		iAttackerMax The maximum number of attackers that can participate in a wave (alive)
//! \param		iDefenderMax The maximum number of Defenders that can participate in a wave (alive)
//! \retval     The desired wave size for the given parameters
//------------------------------------------------------------------------------------------------
int CvUnit::computeWaveSize( bool bRangedRound, int iAttackerMax, int iDefenderMax ) const
{
 
I'm back from my trip though I will be working for a few days before I can get some major code crunching time.

Lord Olleus is getting the same compile error I was having (so its not realy a corrupted file or anything like that). We need to solve this problem not simply so Spoiled Fruit can do a merger, rather we need to make sure the code remains compilable for EVERYONE. It seems the source of this error is in SimCuties code as I dont experience it when compiling myown code or 12monkeys. I am afraid I can't be of much help in debugging it.

GREAT_POPLE_POINT_POOLING should apear on the Enum.h right above NUM_GAME_OPTIONS, I must have removed it misspelled it someware as I did a few other Enums for commerce that you dont seem to have any trouble with these. It can easily be added in, if you dont mind.

On comenting styles, generaly when I change a function and the change involves a simple if statment changing the return value I use.

funtion()
{
//Start Change Mod Name Impaler[WrG]
if (myModisEnabled)
{
return newreturn
}
else
{
//Original Behavior
return original return
}
//End Change
}

I like the way that WinMerge will see this as two seperate blocks of change wrapped around the original code, its shows quite literaly that the original has been wrapped and not itself altered. When I do a single line change I do it like such

//Start Change Mod Name Impaler[WrG]
//ORIGINAL CODE original-line-of-code
my-new-line-of-code
//End Change

I cant see how any diff tool or person would fail to see that as a change.

I think we should try to stick with Firxais style, I have been trying to replicate their style both in the mechanicaly and stylisticly (ofcorse it helps that 90% of my changes are pastes from some other portions of the SDK). I see no real reason to go out of our way to keep changes spacialy confined, I do obvious stuff like putting two function that refference each other right next to each other but nothing besides that, you have to jump all over to understand Firaxis code anyway.

As I've said before I am against Ifdef, I see no value for the end users. The whole point of this project is that the end user is NOT competed at C++ and can not even understand let alone compile the code. The only arguable advatage for us is in debugging which I also find to be slim. Ifdef presents more wrinkles into the equation, makes the code harder to read and in my opionion is a way to excuse sloppy codeing, if its not working dont use it, if its is working it dosn't need an ifdef.
 
Impaler[WrG] said:
I'm back from my trip though I will be working for a few days before I can get some major code crunching time.

Lord Olleus is getting the same compile error I was having (so its not realy a corrupted file or anything like that). We need to solve this problem not simply so Spoiled Fruit can do a merger, rather we need to make sure the code remains compilable for EVERYONE. It seems the source of this error is in SimCuties code as I dont experience it when compiling myown code or 12monkeys. I am afraid I can't be of much help in debugging it.
Resolved in latest update. See code attached above.
Impaler[WrG] said:
On comenting styles, generaly when I change a function and the change involves a simple if statment changing the return value I use.

funtion()
{
//Start Change Mod Name Impaler[WrG]
if (myModisEnabled)
{
return newreturn
}
else
{
//Original Behavior
return original return
}
//End Change
}

I like the way that WinMerge will see this as two seperate blocks of change wrapped around the original code, its shows quite literaly that the original has been wrapped and not itself altered.
It is OK. Source line remain intact. "Start" mark in separate line. But Your actual code did not seems to follow your own rule.
There are many occasion like this:
Code:
    //Addition LocalSpecialistYieldChanges by Impaler[WrG]  5/16/06
    int iI;
    if (m_ppaaiLocalSpecialistExtraYield != NULL)
	{
		for (iI = 0; iI < GC.getNumSpecialistInfos(); iI++)
		{
			SAFE_DELETE_ARRAY(m_ppaaiLocalSpecialistExtraYield[iI]);
		}
		SAFE_DELETE_ARRAY(m_ppaaiLocalSpecialistExtraYield);
	}
	//End
It has no "if (myModisEnabled)" part as I see.
If you didn't use #ifdef, you should use "if (myModisEnabled)" at least.
Use if (myModisEnabled)" on every code change occasion.

And look at this...
Code:
void CyCity::changeExtraSpecialistYieldOfType(int /*YieldTypes*/ eIndex, int /*SpecialistTypes*/ eSpecialist, int iChange)   //Addition SpecialistTechCommerceChange by Impaler[WrG]

Impaler[WrG] said:
When I do a single line change I do it like such

//Start Change Mod Name Impaler[WrG]
//ORIGINAL CODE original-line-of-code
my-new-line-of-code
//End Change

I cant see how any diff tool or person would fail to see that as a change.
As I said, be consistent. Use "if (myModisEnabled)" and use same style with multiline change. There is no reason to treat tem specially.
Impaler[WrG] said:
I think we should try to stick with Firxais style, I have been trying to replicate their style both in the mechanicaly and stylisticly (ofcorse it helps that 90% of my changes are pastes from some other portions of the SDK). I see no real reason to go out of our way to keep changes spacialy confined, I do obvious stuff like putting two function that refference each other right next to each other but nothing besides that, you have to jump all over to understand Firaxis code anyway.
Because it will help to our code separate form Firaxis. It will also help other modder to distinguish original behavior and changed behavior. We should minimize impact on original source code as we can.
Impaler[WrG] said:
As I've said before I am against Ifdef, I see no value for the end users. The whole point of this project is that the end user is NOT competed at C++ and can not even understand let alone compile the code. The only arguable advatage for us is in debugging which I also find to be slim. Ifdef presents more wrinkles into the equation, makes the code harder to read and in my opionion is a way to excuse sloppy codeing, if its not working dont use it, if its is working it dosn't need an ifdef.
SDK code itself is of no value to end user from the start. It is for modder who uses this SDK code. Our target audiance is Python/SDK modder, not end player. Isn't it?
Feature selection will give these moder freedom to choose. It is our mis-conception that other moder will welcome and use all the features we had put in to this project. Let them choose feature whatever they want. It is our obligation to provide them means to do so if they choose to. We should not force-feed our features to all the modder.
This is what all the #ifdef is for.
 
The 2 expamples you site are both Additions not changes. I destinquish a change as something that radicaly changes the return value of a function or cause the function to not perform one of its previous tasks. An Addition dose neither of these things, rather it just adds another things for a funtion to do or adds another value for it to add to a large sum.

Both are taged as such. The First is a critical Array initialization within the Info initializer for an XML controled feature, it would neadlessly complicate things as well as introduce all kind of crash potential to have some kind of if statment or ifdef for a feature that is controled by XML tags in the Units, Tech or Buildings .xml The end user should get a feature which functions as much like a Firaxian function as possible so their is no conditional or game option that turn it off.

The second example is a whole function created from scratch, our comenting style holds that a tag on the first line of a function designates it as a new functions.

On end users: the Python Moders generaly dont know C++ and should not have to dive into it, thats not what they want. They just want nice usable hooks to hang stuff off of, they dont want to worry if the DLL they have is going to work or not because the feature was ifdef'ed out. We are not force feeding anything to anyone, everything is explicity activated by the end user (atleast in my code), I feel XML is the best most easily accessable newb friendly method of control, python is next best and ifdef the LEAST friendly and least practical as we would need to have seperate compiled binaries for every feature activation, it would be a compatability nightmare and pointless extra work for thouse able to compile the code.
 
Impaler[WrG] said:
The 2 expamples you site are both Additions not changes. I destinquish a change as something that radicaly changes the return value of a function or cause the function to not perform one of its previous tasks. An Addition dose neither of these things, rather it just adds another things for a funtion to do or adds another value for it to add to a large sum.
It is YOUR criteria others are not aware of. The addition also incurs some overhead at least in code space and speed. Use consistent style, not ad-hoc style of convenience. It should be minimized. If the feature is not used, it adds dead weight on final DLL. It should be clearly identifiable from source and have minimal overehead.
Impaler[WrG] said:
Both are taged as such. The First is a critical Array initialization within the Info initializer for an XML controled feature, it would neadlessly complicate things as well as introduce all kind of crash potential to have some kind of if statment or ifdef for a feature that is controled by XML tags in the Units, Tech or Buildings .xml The end user should get a feature which functions as much like a Firaxian function as possible so their is no conditional or game option that turn it off.
You can have both XML enabled and feature selection/runtim-config enabled at same time. Having them XML enabled is not excuse for let the dead code left in active state. what is worse, it there is no single point of control the feature. You should edit whole XML to remove every instance of the XML attribute to disable the feature. It is much less flexible.
We should have single point of control to enable/disable specific feature.
Impaler[WrG] said:
The second example is a whole function created from scratch, our comenting style holds that a tag on the first line of a function designates it as a new functions.

On end users: the Python Moders generaly dont know C++ and should not have to dive into it, thats not what they want. They just want nice usable hooks to hang stuff off of, they dont want to worry if the DLL they have is going to work or not because the feature was ifdef'ed out. We are not force feeding anything to anyone, everything is explicity activated by the end user, I feel XML is the best most easily accessable newb friendly method of control, python is next best and ifdef the LEAST friendly and least practical as we would need to have seperate compiled binaries for every feature activation, it would be a compatability nightmare and pointless extra work for thouse able to compile the code.
It is your misconception that other modder don't have ability to code in C++. There are zillions of C++ coder out there. Much more than Python coder ( at least by 10 times as many as C++ coder than Python coder). Do you really belive that there are more Python-only coder than C++ only coder?
This is why I call your assumption as misconception.
They are not XML-only newbie. They are modder who is capable of programming of some kind. Dont underestimate modder here. I found that they are much more capable than myeself, guessing from their python proficency.
We should provide them with "hook" to do what these capable modder can do what they want.
Don't cripple these capable modder for XML-only newbies.
 
SimCutie said:
On commnet style on individual function.
How about to use Firaxis function commneting style, then.

From original Firaxis function. CvUnit.cpp
Code:
//------------------------------------------------------------------------------------------------
// FUNCTION:    CvUnit::computeWaveSize
//! \brief      Computes the wave size for the round.
//! \param      bRangedRound true if the round is a ranged round
//! \param		iAttackerMax The maximum number of attackers that can participate in a wave (alive)
//! \param		iDefenderMax The maximum number of Defenders that can participate in a wave (alive)
//! \retval     The desired wave size for the given parameters
//------------------------------------------------------------------------------------------------
int CvUnit::computeWaveSize( bool bRangedRound, int iAttackerMax, int iDefenderMax ) const
{

This is a very good idea. This style allows you to Doxygen the source code:

http://www.stack.nl/~dimitri/doxygen/

It's what I've been using for my mod. I've attached an early version of one page that Doxygen generated.
(I'm currently working with the Warlords SDK, and have cleaned it up a bit, but would probably get into trouble if I posted unreleased source code.)

It'll give you an idea of what it does but it doesn't allow you to see how easily you can browse through the Doxygened files.
 

Attachments

Ok this is simply so unreasonable I am having trouble remaining calm in my reply. Everyone else I have talked with about this is fine with XML control only, run-time in addition is both unessary and ill advised. I have added 3x6 int matricies to City and Player, these objects have HUNDREDS of data member already, the overhead both in memmory and speed is piss in the ocean. All the combined changes of this project together add .001 MB to the DLL, our changes are like fleas on an Elephant! Overhead will not become an issue untill we get into deep AI re-programing and no I do not see a need to switch AI improvments at run time, no one would want that.

SimCutie I ask you to remember who your fellow project members are, we are mostly amatures working in our spare time hacking away at Firaxis work. Do not smother them with standards and practices that are inappropriate for the scale of this project. Dose anyone else agree with me on this point?
 
Impaler[WrG] said:
Ok this is simply so unreasonable I am having trouble remaining calm in my reply. Everyone else I have talked with about this is fine with XML control only, run-time in addition is both unessary and ill advised. I have added 3x6 int matricies to City and Player, these objects have HUNDREDS of data member already, the overhead both in memmory and speed is piss in the ocean. All the combined changes of this project together add .001 MB to the DLL, our changes are like fleas on an Elephant! Overhead will not become an issue untill we get into deep AI re-programing and no I do not see a need to switch AI improvments at run time, no one would want that.

SimCutie I ask you to remember who your fellow project members are, we are mostly amatures working in our spare time hacking away at Firaxis work. Do not smother them with standards and practices that are inappropriate for the scale of this project. Dose anyone else agree with me on this point?
Hey Impaler, calm down and ask yourself. What is worng with my point? It doesn't matter at all here even if I am a dog and your are Mr. Bill Gates. You should answer with logical persuation, not anger or personal attack.
OK. overhead can be tolerated if it is not excessive. But having single point of control for given feature is widely supported convention of most open source software. Choose any respectable open source software and look at their code. They have lots of #ifdef to select featrues. It is not standard of commercial software, but standard of open-source software.
Why they do so? Overhead is not only reason they are doing so.
It is matter of freedom. freedom to choose.
Moder should have simple means to deactivate feature they don't want. That decision should be made by the modder, not by us. Let them choose what ever suits their need. It is pivotal characteristic of sucesss of open-source software.
 
Just to clarify, you are talking about modding features, rather than features that we do want runtime config on... like pretty much anything we have at the moment that isn't a modding feature.

I'm personally not too keen on the #ifdefs, and it doesn't seem that anybody else much is either. I don't see the overhead brought about by adding modding features to be that great when compared with the overhead of what is already there. Deactivating a feature will have no effect other than to reduce whatever overhead it produces, which, as I said above, should be minimal. XML control (where nothing there means the option is disabled) seems far simpler and more accessable to me. A modding feature doesn't do anything if the user doesn't use it.

This is not really like other open-source software either. From what I can tell the majority of modders aren't going to open up the SDK and disable the option. They won't have to, because it won't be enabled in the first place.

Personally, I think you're overestimating the talents of most Civ 4 modders. If you look at the large modpacks which are out there much of the code has been done by people who hadn't written a single line of code before Civ. We're opening up more options for the easy stuff rather than forcing them to delve into the hard stuff.
 
12monkeys said:
What is the actual version, what should I DL for testing? Anything I missed?
I think the current version is on the download thread. From what I can gather, it doesn't work, and crashes as SF described.
 
Impaler[WrG] said:
Do not smother them with standards and practices that are inappropriate for the scale of this project. Dose anyone else agree with me on this point?
We do need some standards, otherwise it's never going to work, and it's going to come out a mess. Personally, I wouldn't mind if everybody just kept to the commenting style, and tried to set out their code like Firaxis have set out theirs.

About DOXY - looks like a good method for documenting new functions. Is anybody against me putting this in the contributions guide (in the vain hope that somebody will read it ;))
 
SimCutie said:
OK. I will change #inclide position to Top of the file as Firaxis's way.
Instead, I suggest use of # ifdef for all our code be adpoted as general guide line.
Resolving overlapping #ifdef is exactly the work we should have to do.
The #ifdef section is not random peice of code. It is logical unit of change for a feature. So if same potion of code should be changed diffrently by combination of feature, it should be logically resolved depending on combination of feature. Not using #ifdef is hiding problem under carpet, not solving it. That is not right thing to do.
Current problem of merged version (by SF) is also same problem. Each piece of code worked without problem before merging. But when they are combined, it caused problem. So it is our task to solve it logically. This is what "integration" is for. if all code used #ifdef for each individual feature, the integration test will be much eaiser, not harder.
Most major open-source software developed by multiple developer adopts such convention (i.e. compile time feature selection by #ifdef adopted by apache web server)

SimCutie, I think the people who will to use the result of our work are either python modders, not able or willing to change or compile the SDK, or they are end users. You can't compare our product with an apache server, or something similar.
You're right, that testing might be easier when we have compiler switches, but in ONLY in one case :
- when we've problems in the merged version you can switch off different features and test if problem persists.
But this you have to buy with certain disadvantages :
- if there is a fault in the #ifdefs, you gain extra problems.
- if you have overlapping #ifdefs, you have to resolve the code. This is time consuming.
- the code is not that easy to read
For me there are more cons then pros it useing #ifdefs. If you like to use them, do so. They doesn't disturb anything what I'm doing, as long as they are all set active. But, PLEASE, don't start make your features as #ifdef options. Will say : if switch A than features X is active, if not feature Y is active.
Somehow, I'm getting tired of that discussion.

12m
 
The Great Apple said:
Personally, I think you're overestimating the talents of most Civ 4 modders. If you look at the large modpacks which are out there much of the code has been done by people who hadn't written a single line of code before Civ. We're opening up more options for the easy stuff rather than forcing them to delve into the hard stuff.

Quoted for TRUTH!

The best mod creators are not necessarily the best programmers. If you want this project to lead to more innovative mods, make as much as possible editable with XML.

For this reason IMHO Impaler has the most useful modifications in the first release. :goodjob: I'm especially looking forward to the Universal Building Modifier in a future release.

Also while Python, I think this:
[SDK]GetXMLElement() extension to CvEntities. Allow from Python to grab an arbitrary XML element for a particular entity, allowing modders to add arbitrary additional xml elements to buildings, units, etc.
will be the holy grail for better modmaking. :)


Maniac - tester
 
Back
Top Bottom