First release

[The Great Apple]

More conventional way:
def EventHandlerRegister(evt_mgr, unused):
       evt_mgr.addEventHandler('Init', onInit) 
       evt_mgr.addEventHandler('UnInit', onUnInit, evt_mgr.PRIORITY_HIGH)

Radical way:
def EventHandlerRegister(evt_mgr, unused):
      return { 'Init' : onInit ,  'UnInit+': onUnInit }

Actually I prefer the radical way.

I'm getting slightly confused here. Does this method actively search for mods with events, or do you have to define them in one specific file? The note at the top of CvSimCutieEventManager.py seemed to imply that it would search for any event manager in the mod file, and use these.

It should install event handler for only events that the mod has need to handle. Leave alone for event that the mod doesn't care. The list itself is readily available in any modder's and player's installation folder.

I disagree. While what you are saying is true, that doesn't mean it's the right thing to do. There are many advantages of having all your events and functions listed out - you can see at a glance what all the events available to you are, and what functions each event calls. If there is a none result, then nothing gets handled, and there is no problem. It's clear, easy to understand, and easy to alter. It may be less clever, or neat, but I think it's better.

It would perhaps be even better if they could attach multiple functions in a list inside the dictionary, so that it's even clearer. For example:

def EventHandlerRegister( evt_mgr, unused = None ):
	EventHandlerMap = {
		...
		'EndGameTurn'		: handleEndGameTurn,
		'BeginPlayerTurn'	: (handleBeginPlayerTurn1, handleBeginPlayerTurn 2),
		'EndPlayerTurn'		: None,
		...
			}

	return EventHandlerMap

This would just be a case of plugging the functions that you want to use on an event into the template. You would never have to change the names of the events, or alter the order, unless you added one yourself. Very easy IMO.

As mentioned above, I think we should make it as clear as we can to the user how to use the mod. This is the clearest method I can think of - it has the names of the events, and the names of the functions so it you can see at a glance what happens when. It has the names of the events you can use, so you add new functions at a glance.

 

[The Great Apple]

Have updated my above post, emphasising some points. We need to get this cleared up to everybodies satisfaction rather soon I think.


On date format:
Yes, I agree dd/mm/yy and mm/dd/yy are very easy to get confused. I will update the commenting guidelines.

 

[SimCutie]

Actually I prefer the radical way.

I'm getting slightly confused here. Does this method actively search for mods with events, or do you have to define them in one specific file? The note at the top of CvSimCutieEventManager.py seemed to imply that it would search for any event manager in the mod file, and use these.

Yes, It actively searches for specific directory like {Custom, MODS/<active-mod-name>/}Assets/Python/Screens and its sub-directories recursively for all python file that has EventHandlerRegister fucntion defined..
Each mod should have its own EventHandlerRegister(). EventManger will call each of the EventHandlerRegister() function in each python files. (It is possible that single mod has multiple EventHandlerRegister in different python files). Each EventHandlerRegister() needs to register only handlers that the specific mod wish to handle.

I disagree. While what you are saying is true, that doesn't mean it's the right thing to do. There are many advantages of having all your events and functions listed out - you can see at a glance what all the events available to you are, and what functions each event calls. If there is a none result, then nothing gets handled, and there is no problem. It's clear, easy to understand, and easy to alter. It may be less clever, or neat, but I think it's better.
...<snip>
As mentioned above, I think we should make it as clear as we can to the user how to use the mod. This is the clearest method I can think of - it has the names of the events, and the names of the functions so it you can see at a glance what happens when. It has the names of the events you can use, so you add new functions at a glance.

It is so if there is only one EventHanderRegister and only one place to add new event handler throughout entire collection of MOD's.
The design of this scheme is exactly opposite.
In old CustomEventManager, there is central pace to add new mod with event handing code. So such centralized list makes sense.
This event manager is just opposite. it has completely de-centralized structure. Every mod that need event handling should have its own EventHandlerregister() in its own mod python file, not in single, big, comprehensive list of mod or event handlers. There is no such central list of all mod or event handlers list in source level. Complete list of event and custom event handlers are available only in run-time and is printed in "PythonDbg.log".
So each EventHandlerMap in specific mod will have only few (2-4) non-empty handlers out of 60+ events in the Map. It is just waste of space.
Most complex mod of mine had only 5 custom event handlers.
Empty entry in the EventHandlerMap serves no purpose at all.

PS) Registering multiple handlers for a same event in single mod is possible and will cause no conflict at all. All handlers are treated equal except for its priority. There is no diffrence between handlers from same mod or from other mod. In fact, there is no way to tell to what mod a specific python file with EventHandlerregister belongs among multiple mod's in action.

 

[The Great Apple]

Yes, It actively searches for specific directory like {Custom, MODS/<active-mod-name>/}Assets/Python/Screens and its sub-directories recursively for all python file that has EventHandlerRegister fucntion defined..
Each mod should have its own EventHandlerRegister(). EventManger will call each of the EventHandlerRegister() function in each python files. (It is possible that single mod has multiple EventHandlerRegister in different python files). Each EventHandlerRegister() needs to register only handlers that the specific mod wish to handle.

Right I thought so. This is good

I'm still slightly concerned about the ease of finding the right string to use to catch each event, though I may be slightly underestimating modder's ability to catch onto these things.

I have a plan - when we finish this build and send it out to some third-party testers we should flag this up as a potential issue. If they understand it then that's great, otherwise we may have to tweak it.

 

[SimCutie]

Right I thought so. This is good

I'm still slightly concerned about the ease of finding the right string to use to catch each event, though I may be slightly underestimating modder's ability to catch onto these things.

I have a plan - when we finish this build and send it out to some third-party testers we should flag this up as a potential issue. If they understand it then that's great, otherwise we may have to tweak it.

Don't worry. If event manager gets wrong event name or invalis handler, then error log is printed in "PythonDbg.log". And complete list of active custom event handlers are printed in the log file.

 

[12monkeys]

On style of canXXXX() hooking..
When you hook some existing canXXX predicates (like CvUnit::canMoveInto())
You use two separate Python entrypoint..( eg. "canMoveInto", "canMoveInto2")
It will confuse modder and has two times of overhead of calling python.
Python call is very expensive call (big overhead) so it should be avoided when Python "canMoveInto" is not used.
It can be improved for better way.
...
...
cm_hasCanUnitMoveInto is class static member(== global) boolean variable.

I really like your idea with this wrapper function. I will review my changes and will rework them regarding this point.

You're using some methods and constants I can't find somewhere else. Where did you take them from? Could it be, that you heavily using code from your own changes?
I have to admit that I understand the concept of your code (will say : what you intend to do), but I don't understand what you'e doing in detail without analysing your changes. It seems to me, that you implemented a very generic approach of those python loopback calls.
I think I'll wait for your final code and rethink your generic approach again then. In between I will use the static calls which requires the functions in the CyGameUtils as it is now.

PS2: I used "canUnitMoveInto" instead of "canMoveInfto" because CvSelectionGroup has same canMoveInto().
I recommend change other Entry point names more descriptively. (ex: "canWork" -> "canWorkOnPlot" )
You should not expect general Python modder will read SDK code. So no need to match both name sacrificing readability/undertandability of Python code.

I had the same doubts at the beginning, but the CySelectionGroup::canMoveInto is just calling the CyUnit::canMoveInto function in loop for all members of the group. This is similar for some other functions. So I choosed the naming conventions of Firaxis and added the source of the call in the comments of the python functions. But I'm still not sure if this is the best solution. Maybe some other opinions to this point?

PS3: Some Python entrypoint predicate canXXX like canStartMission()/canTradeWith()/canFound()/canConvert() returns TRUE, FALSE, or -1. -1 for obeying original code result. It is confusing.
Python canXXX() should return always boolean value. not -1.
No other Firaxis canXXX() Python entry point uses such questionable practice.
You can use same scheme described above to make them return only boolean value.

Yepp! Thats surely a dirty solution. but it was the simplest way to have a tri-state return. But you're right, I can replace it by passing the SDK decision to Python so that the user can return it if he don't wants to change something. This is surly better. Good call!

12m

 

[12monkeys]

@SimCutie :
BTW : where is the difference between

	const CyUnit* pyUnit = new CyUnit(const_cast<CvUnit*>(this));

and

	const CyUnit* pyUnit = new CyUnit((CvUnit*) this);

As I understand there is no difference, but I'mnot sure.

12m

 

[12monkeys]

... I am just a hobbist in modding. ....

What do you think we are? Professional modders ?
TBH : When I look for the code you writing and the concepts you're implementing you should look for a job as professional programmer. And I mean this seriously.

... I should have much thing to learn from you.

Don't overestimate me.

... But our project is not commercial product. So I think that applying or demending strictly same practice and standard of quality like commercail product to our project is too much or just overkill and counter-productive. More over Civ4 is just a game, not mission-critical or life-supporting software of spaceship.
To accomplish such degree of quality of code and documentation, we would hardly make out any notable thing before next expansion come out! and the next expanison is following us, not far from our back.

Yea, thats true. We're not an commercial project and thats good for sure. Otherwise it woudn't make so much fun.
I also don't expect that everything is error free or perfect. I don't even expect that form our programmers at work. But nevertheless this project has targets and one target is that other people should use what we are doing. If they don't use it, we've failed. Thats my only concern.

 

[SimCutie]

@SimCutie :
BTW : where is the difference between

	const CyUnit* pyUnit = new CyUnit(const_cast<CvUnit*>(this));

and

	const CyUnit* pyUnit = new CyUnit((CvUnit*) this);

As I understand there is no difference, but I'mnot sure.

12m

It is just purely of cosmetic difference.
Former one is ANSI C++-98's way of casting const to non-const. quite selectively.
Latter is old C's way of casting anything to another thing.
I used former just to have false feeling of comformance to latest standard of C++ , a kind of fanfaronade or techno-snobism .
In fact, Old C's way is as good as that of C++.

 

[SimCutie]

I really like your idea with this wrapper function. I will review my changes and will rework them regarding this point.

You're using some methods and constants I can't find somewhere else. Where did you take them from? Could it be, that you heavily using code from your own changes?
I have to admit that I understand the concept of your code (will say : what you intend to do), but I don't understand what you'e doing in detail without analysing your changes. It seems to me, that you implemented a very generic approach of those python loopback calls.
I think I'll wait for your final code and rethink your generic approach again then. In between I will use the static calls which requires the functions in the CyGameUtils as it is now.

Yes, I have a library for general use in Civ4 SDK modding. (CvExtension.cpp) Until recently better part of my time was spent on building the library / infrastructures rather than actual modding. I like this approach. With good and stable building blocks, future job will be much easier.
Geneic call is not much thing. It is just couple of funtions. It is used to save typing of C++ code ( I am slow and error-prone typist) and reduce number of code lines inserted between Firaxis code. With less impact on original source code, it will be easier to migrate our code to next Expansion("Warlord") version of SDK. I recomand you not to scatter your changes all over the source codes. I ususally concentrate them in end of existing source file.
The library is quite new and evolving rapidlly so I think that it is not time for other modder to depend on it. You can wait until it is cooked well.

I had the same doubts at the beginning, but the CySelectionGroup::canMoveInto is just calling the CyUnit::canMoveInto function in loop for all members of the group. This is similar for some other functions. So I choosed the naming conventions of Firaxis and added the source of the call in the comments of the python functions. But I'm still not sure if this is the best solution. Maybe some other opinions to this point?

Please comapre your names with other function names in GameIntarface.py. Firaxis function names are generally longer and descruptive. Follow naming conventions used by Firaxis one.
The names are very important because it is part of Python mod API which should not be changed later once it is decided now for backward compatibility with Python mod which depens on it.

Basically, Python modder should be able to figure out what the function does from its name in the GameInterface/GameUtils functions without reading long document or SDK code. This is why the Firaxis functions have somewhat longer and descriptive names. I like name form like <verb>+ <objective> style. ie. canWorkOnPlot() instead of canWork().

 

[The Great Apple]

Right.

It would seem that we've just about got everything sorted. Does anybody have an objection to seeing if we can get the first version testing tomorrow?

For this we will need people to upload onto the server everything they currently have which is working & complete, and which they want included. I also need a change log from everybody uploading stuff.

A quick reminder - remember that the default config should leave the game the user sees exactly as it was before the user installed the mod, except the new options menu.

 

[Impaler[WrG]]

I never managed to connect to Spoiled Fruits Server, with the CVS finaly cleared out I thought we would be using that instead. I will upload my Code to the forum here and hope Spoiled can put it in my Folder (I will get in their someday I hope).

 

[The Great Apple]

I never managed to connect to Spoiled Fruits Server, with the CVS finaly cleared out I thought we would be using that instead. I will upload my Code to the forum here and hope Spoiled can put it in my Folder (I will get in their someday I hope).

Did you try the new IP he posted here? The old one went down due to a power failure.

 

[12monkeys]

Did you try the new IP he posted here? The old one went down due to a power failure.

This new one is down as well, so I'll UL my version here. Its finished so far.

The difference to the first one are :
- date format in the comments is changed to format dd-MMM-yyyy
- implemented some wrappers as SimCutie proposed
- renamed the python functions in game utils to more clear names

Its tested so far and can be implemented as soon as there is something to merge.

12m

 

[The Great Apple]

Right, anybody else?

EDIT: In other words, if you have finished could you upload your code, otherwise could you post an ETA of when you will be finished?

 

[SpoiledFruit]

Sorry but I had to reset the entire local domain here. It works with the new IP now (Also for some weard reason the HD with the FTP data didn't get detected by the right after the power outage. It works fine now).

 

[SimCutie]

Here is my version.
Ready to intergrate into project.

 

[Impaler[WrG]]

Here is my code ready to merge, if any of the documentation is unclear please ask and I can in more detail describe the changes. As most effects require XML to activate and test I will prepare some content files that you can slip in Custom Assets, make shure your using the main assets pack here as your explicitly loaded mod as it contains the nessary Schemas to alow thouse content files to be read.

 

[The Great Apple]

Okies - I think these are the only three ones we're going to get for this release.

@ SimCutie
Browsing through your changelong I am a bit concerned by the following:

1.4  When Unit name was set by player, the unique(custom) name returned by 
	CvUnit::getName() will be "Unit Type, custom name"
	This is run-time config (UNITNAME_TYPEFIRST) and turned on/off in custom Option Screen.
	NOTE: I tried to put it as PlayerOption but PlayerOption is not working properly as of 1.61
	It will be changed to PlayerOption if this is fixed in later Civ4 version.
	Old : "My best defender (Archer)"
	New : "Archer, My best defender"
	Behavior on unit without unique / custom name is not changed.

1.6 Added Great People Type and Civs name in Great People born announcement.
	Old: "Albert Einstein has born in Frankfurt"
	New: "Albert Einstein (Great Scientist) has born in Frankfurt, Germany" 

1.7 Added Unit Type name in press-AltKey Combat Odd display
	Old: "4.3 vs. 6.1"
	New: "4.3 (Axeman) vs. 6.1 (LongbowMan)"

Firstly, 1.4 isn't a problem, except from what I can tell it is enabled by default. Which it should not be.

1.6 and 1.7 should also be configurable, and disabled by default. Maybe make one flag - something like "Enhanced text info", and have it trigger all three?

Also - what does

CvUnit Script data / experience is preserved across unit upgrade/ unit gift.

mean? On the same entry - I think it may have been originally intended for foritifcation to be broken when updrading. I'm not sure we should change this.

We have to be careful not to change the game in any way the user might not like without the user's permission.

 

[SimCutie]

Okies - I think these are the only three ones we're going to get for this release.
.............
EDIT: Am also updating the changelog. Will change this bit when I'm done.

OK. All will be done soon. And You are right, not experice, but fortification.. it is documentation error. Experience is preserved even in original version.