• In anticipation of the possible announcement of Civilization 7, we have decided to already create the Civ7 forum. For more info please check the forum here .

OOS Discussion, Tracking and Fixing

Latest game

Elohim password = 'j'
Bannor password = 'aa'

Everytime either of us double clicks on a city the game out of syncs
100% chance
 

Attachments

  • SMIFFGIG AD-0278.CivBeyondSwordSave
    429.8 KB · Views: 131
Someone (Tasunke, maybe?) said that differing BUG options can sometimes cause OOS issues.

I can double check but niether of us have has touched BUG or any of the options in it they are the same.

Furthermore if it helps as I mentioned before, game killing OOS have happended only on the 3 (4?) games we have had as two players on the same team (we have always been Bannor and Elohim on these games)

It hasnt always been the same as above (where it OOS only when you access the city screen)

If there are any more useful details I could provide then let me know
I can also dig out the other OOS save games if you need them?

Edit (quote regarding Hyborem OOS)
Interestingly enough, the function even has a warning about OOS issues.

Anyway, it seems that one solution might be to simply move this code somewhere else. However, if possible, I'd like to find the root of the problem and fix it there. the CityAcquiredandKept function is called quite often, so I'm curious if other Infernal activities also cause OOS issues. What if a city is traded to the Infernals? What if they capture and raze a city?

In the latest game mentioned above where the game OOS for either allied (Team 1) player if they enter a city seemed to happen shortly after (but not straight away.. a few turns) I (Elohim) took a Clan of Embers city.
Maybe this is related or just coincidence?

Is there anyway to properly track what actually is happening to cause these out of syncs?
 
I have been trying to debug the Kuriotate OOS issue again. After lurking the forums a bit I found that other mods such as C2C include a special OOS logger file (https://svn.code.sf.net/p/caveman2cosmos/code/trunk/Assets/Python/OOSLogger.py) and, to my surprise, found that MNAI already includes it. I did not manage to make it work (the text file is never generated after an OOS) and it seems to be intended to be called each turn (onGameUpdate in CvEventManager, although I'm not sure if that is actually called each turn).

I tried to see if it is being called by adding a python error on purpose to OOSLogger.doGameUpdate, but I saw nothing in the pythonerr log. Anyone has any ideas to make logging work?
 
I have been trying to debug the Kuriotate OOS issue again. After lurking the forums a bit I found that other mods such as C2C include a special OOS logger file (https://svn.code.sf.net/p/caveman2cosmos/code/trunk/Assets/Python/OOSLogger.py) and, to my surprise, found that MNAI already includes it. I did not manage to make it work (the text file is never generated after an OOS) and it seems to be intended to be called each turn (onGameUpdate in CvEventManager, although I'm not sure if that is actually called each turn).

I tried to see if it is being called by adding a python error on purpose to OOSLogger.doGameUpdate, but I saw nothing in the pythonerr log. Anyone has any ideas to make logging work?

I had a look at it. It seems like whoever wrote BugEventManager did not fully understand how class method overrides work in Python. As a result, some event handlers get replaced with duplicates of other handlers. One of those handlers is the one for the 'gameUpdate' event.

A clean solution would basically require rewriting BugEventManager almost completely. A quick and dirty fix is to edit CvEventManager instead, to safeguard it from the shenanigans of BugEventmanager.

If you are going for the easy route, you need to edit CvEventmanager.py as follows:

Find the part that looks like this:
Code:
		## EVENTLIST
		self.EventHandlerMap = {
			'mouseEvent'			: self.onMouseEvent,

Change the 'mouseEvent' line to look like this:
Code:
			'mouseEvent'			: CvEventManager.onMouseEvent.__get__(self,CvEventManager),
Then do the same for the next 80 or so similar lines. It really helps if you use a regex-capable search & replace tool.

The next part is not strictly necessary; just a little future-proofing:
Change all occurences of "self.beginEvent(" with "CvEventManager.beginEvent(self,".

You probably also want to replace OOSLogger.py with the one from C2C that you linked to, as it contains a little more functionality.


A quick test with an old save that reliably produces OOS events revealed something peculiar.
The two human players are Amurites and Kuriotates. One might expect the Kuriotates player to be the problem, but the OOS log indicates that the discrepancy is with one of the AI players (in this case player 8, Doviello).
I have attached both OOS logs and the save to this post.
 

Attachments

  • fe79 - Player 0 - OOSLog - Turn 100.txt
    374.8 KB · Views: 137
  • Admin2 - Player 1 - OOSLog - Turn 100.txt
    374.8 KB · Views: 82
  • amu_kur 3.CivBeyondSwordSave
    160.7 KB · Views: 129
I had a look at it. It seems like whoever wrote BugEventManager did not fully understand how class method overrides work in Python. As a result, some event handlers get replaced with duplicates of other handlers. One of those handlers is the one for the 'gameUpdate' event.

A clean solution would basically require rewriting BugEventManager almost completely. A quick and dirty fix is to edit CvEventManager instead, to safeguard it from the shenanigans of BugEventmanager.

If you are going for the easy route, you need to edit CvEventmanager.py as follows:

Find the part that looks like this:
Code:
		## EVENTLIST
		self.EventHandlerMap = {
			'mouseEvent'			: self.onMouseEvent,

Change the 'mouseEvent' line to look like this:
Code:
			'mouseEvent'			: CvEventManager.onMouseEvent.__get__(self,CvEventManager),
Then do the same for the next 80 or so similar lines. It really helps if you use a regex-capable search & replace tool.
The next part is not strictly necessary; just a little future-proofing:
Change all occurences of "self.beginEvent(" with "CvEventManager.beginEvent(self,".
Thanks for looking into that and solving that issue, but I think your solution is not perfect, because you misunderstood some things.
BugEventManager wasn't written for FfH, but for vanilla BtS as part of the BUG mod. Based on what you say, it seems the problem is that it overrides some functions that were changed in FfH with BtS+BUG versions.

For better illustration: onGameUpdate in FfH looks like that:
Code:
def onGameUpdate(self, argsList):
	'sample generic event, called on each game turn slice'
	genericArgs = argsList[0][0]	# tuple of tuple of my args
	turnSlice = genericArgs[0]
#FfH: 10/15/2008 Added by Kael for OOS logging.
	OOSLogger.doGameUpdate()
#FfH: End add
The code commented with #FfH... was obviously added in FfH. So in BtS the function did exactly nothing, and BUG could override it.

Did I understood right that you want to keep BugEventManager completely to override EventManager methods? I guess that would break some BUG features.

A part of the BUG system is designed to keep modders from modifying CvEventManager directly and put there functionality in BUG modules (python modules with some XML configuration to for event handling). I think most BUG-based mods don't modify CvEventManager; and MNAI should probably put all it's event manager functionality into a module.

See this for more information about BUG modules (They call it "mods on the BUG platform).

And yeah, nice contribution, especially for a first post :).
 
Thanks for looking into that and solving that issue, but I think your solution is not perfect, because you misunderstood some things.
BugEventManager wasn't written for FfH, but for vanilla BtS as part of the BUG mod. Based on what you say, it seems the problem is that it overrides some functions that were changed in FfH with BtS+BUG versions.

For better illustration: onGameUpdate in FfH looks like that:
Code:
def onGameUpdate(self, argsList):
	'sample generic event, called on each game turn slice'
	genericArgs = argsList[0][0]	# tuple of tuple of my args
	turnSlice = genericArgs[0]
#FfH: 10/15/2008 Added by Kael for OOS logging.
	OOSLogger.doGameUpdate()
#FfH: End add
The code commented with #FfH... was obviously added in FfH. So in BtS the function did exactly nothing, and BUG could override it.

It did nothing, which is probably why the problem went unnoticed. Also, there are other handlers that actually do things, even in vanilla BTS.


Did I understood right that you want to keep BugEventManager completely to override EventManager methods? I guess that would break some BUG features.

When BugEventManager initializes, all the event handlers from CvEventManager are explicitly imported, so the intention was clearly that the old handlers should still be active, and new ones just added to them. Some handlers were implemented in methods that are overridden in BugEventManager, which leads to a situation where the old handler is never called, and the new handler is called twice. The fix ensures that both old and new handlers are called, one time each.

The gist of the problem is that CvEventManager was not designed to be a base class, and that is the problem that the fix targets.

As I said in my first post, it is a quick and dirty fix. Redesigning BugEventManager would be better, but would also be more work.


A part of the BUG system is designed to keep modders from modifying CvEventManager directly and put there functionality in BUG modules (python modules with some XML configuration to for event handling). I think most BUG-based mods don't modify CvEventManager; and MNAI should probably put all it's event manager functionality into a module.

That would be one way to go, but the fix is still necessary to avoid some handlers being called twice for each event.
 
It did nothing, which is probably why the problem went unnoticed. Also, there are other handlers that actually do things, even in vanilla BTS.

When BugEventManager initializes, all the event handlers from CvEventManager are explicitly imported, so the intention was clearly that the old handlers should still be active, and new ones just added to them. Some handlers were implemented in methods that are overridden in BugEventManager, which leads to a situation where the old handler is never called, and the new handler is called twice. The fix ensures that both old and new handlers are called, one time each.
I thought BugEventManager doesn't overrides these, or if it does, it includes the functionality of vanilla BtS, but I just checked and you're right. Thanks for the explanation.
The gist of the problem is that CvEventManager was not designed to be a base class, and that is the problem that the fix targets.

As I said in my first post, it is a quick and dirty fix. Redesigning BugEventManager would be better, but would also be more work.
You mean f.e. renaming the methods BugEventManager overrides in BugEventManager itself so it no longer overrides the ones in CvEventManager?

And your dirty fix still lets the BugEventManager methods be called, right, since they are added in BugEventManager.__init__()?
That would be one way to go, but the fix is still necessary to avoid some handlers being called twice for each event.
Yeah, I understand now, the problem seems to lie in BugEventManager

But I'm still uncertain if we're not both overlooked something; first, because BUG is used in so many mods and therefore should be quite stable, second, because in CvEventManager.onKbdEvent there is some FfH code to rotate the camera with Shift+Right/Left which works. That functionality may be added somewhere else of course.

EDIT: I just tested onKbdEvent, BugEventManager clearly overrides the method of CvEventManager (would have been odd if not :)), but onKbdEvent is only sometimes called twice. If a shortcut like ALT-X for the dotmap is recognized, it isn't, and I think that is related to _handleConsumableEvent(), which is mapped on kbdEvent with EVENT_FUNCTION_MAP at line 543 of BugEventManager. Still, the double call is obviously not intended, while it probably remained unnoticed that way. Also as far as I can see only two functions are overridden, onKbdEvent and onGameUpdate. I'm not sure whether overriding the former function was intended or not, since BUG provides a new shortcut mechanism.
You should post that in the main Civ4 Creation&Customization forum, other mods might be interested and maybe already fixed that.
 
You mean f.e. renaming the methods BugEventManager overrides in BugEventManager itself so it no longer overrides the ones in CvEventManager?

That would also work, but it is a fragile solution. Whenever you want to add a new method to BugEventManager, you would have to inspect CvEventManager to make sure you are not overriding something by mistake.


And your dirty fix still lets the BugEventManager methods be called, right, since they are added in BugEventManager.__init__()?

Yes. The only difference in that regard is that the double-call problem is gone.


But I'm still uncertain if we're not both overlooked something; first, because BUG is used in so many mods and therefore should be quite stable

Only a few events are affected, and vanilla BTS does not do anything important in those event handlers.


second, because in CvEventManager.onKbdEvent there is some FfH code to rotate the camera with Shift+Right/Left which works. That functionality may be added somewhere else of course.

A quick test seems to indicate that this functionality might be hard-coded in the game engine.


EDIT: I just tested onKbdEvent, BugEventManager clearly overrides the method of CvEventManager (would have been odd if not :)), but onKbdEvent is only sometimes called twice. If a shortcut like ALT-X for the dotmap is recognized, it isn't, and I think that is related to _handleConsumableEvent(), which is mapped on kbdEvent with EVENT_FUNCTION_MAP at line 543 of BugEventManager.

Consumable events are handled slightly differently than other events. For most events, all registered event handlers are unconditionally called in sequence. For consumable events, they are called in sequence until one returns a non-zero value. Thus, the double call is not seen when the duplicated handler actually consumes the event.


Still, the double call is obviously not intended, while it probably remained unnoticed that way. Also as far as I can see only two functions are overridden, onKbdEvent and onGameUpdate. I'm not sure whether overriding the former function was intended or not, since BUG provides a new shortcut mechanism.

Keeping the old keyboard event handler only affects the new shortcut mechanism if there is a collision in the set of key combinations they handle. If there is such a collision, I think the old handler gets priority by virtue of being added first.
 
fe79: Awesome! Welcome to the forums, and thank you for figuring the cause of the problem :)

It will be some time until I can have a look at this problem again. I will start by using your advice (and lfgr's) to fix the creation of OOS logs.

The difference in that log is unexpected... it seems to be caused by a different selection of tiles to work by some Doviello city. At first glance this seems unrelated to the presence of the Kuriotates..
 
When merging BUG, I was unable to get the modular CvEventManager scheme to work for me. I tried moving the FFH stuff into its own file but it was never accessed. It's possible that I didnt add the proper XML configurations that lfgr mentioned. Anyway, I mucked around with it quite a bit and its possible that I changed something along the way that broke some things. There are some BUG features that don't seem to be working properly (CivAlerts for example).

If anyone wants to look into it further, I would be more than happy to accept the help and advice!
 
When merging BUG, I was unable to get the modular CvEventManager scheme to work for me. I tried moving the FFH stuff into its own file but it was never accessed. It's possible that I didnt add the proper XML configurations that lfgr mentioned. Anyway, I mucked around with it quite a bit and its possible that I changed something along the way that broke some things.

I have had a look at the "pure" BUG mod. It has the exact same code, and thus the exact same problem. It was not something you caused. It simply affected your mod more because you used the old event system.


There are some BUG features that don't seem to be working properly (CivAlerts for example).

If anyone wants to look into it further, I would be more than happy to accept the help and advice!

You need just a few tweaks:

To get it (and other things) working in single-player and non-networked multi-player, you need to undo one of Kael's old changes. When he added the OOS-logger, he also restricted the gameUpdate event to only fire in networked multi-player games. I do not know why he did that, since the only code that was affected at the time was the OOS-logger, which only activates when the game goes out of sync (which of course can only happen in networked multi-player games).
In any case, the part you need to change is in CvGame::update(), in CvGame.cpp:
Code:
//FfH: Modified by Kael 10/15/2008
//		// sample generic event
//		CyArgsList pyArgs;
//		pyArgs.add(getTurnSlice());
//		CvEventReporter::getInstance().genericEvent("gameUpdate", pyArgs.makeFunctionArgs());
        if (isNetworkMultiPlayer())
		{
            CyArgsList pyArgs;
            pyArgs.add(getTurnSlice());
            CvEventReporter::getInstance().genericEvent("gameUpdate", pyArgs.makeFunctionArgs());
        }
//FfH: End Modify
Just get rid of the part that is not commented out, and uncomment the part that is commented.


Next, you need to actually enable Civ4lerts. This is done by setting Enabled = True in <User>\Documents\My Games\Beyond the Sword\FFH - More Naval AI\Settings\Civ4lerts.ini.
You may also want to edit Assets/Config/Civ4lerts.xml:
Code:
	<options id="Civ4lerts" file="Civ4lerts.ini">
		<section id="Civ4lerts">
			<!-- Move this outside of INI once Mod objects hold options -->
			<option id="Enabled" key="Enabled" 
					type="boolean" default="False" 
					get="isEnabled" set="setEnabled" 
					label="Enable Civ4lerts" 
					help="When checked, messages are displayed to alert you to various pending and existing conditions using Dr. Elmer Jiggle's Civ4lerts mod."/>
Change to default="True" to make it active by default.


I have only had time for a few quick tests, but with these changes I at least got messages like "$City will grow to size $NewSize next turn" and some others.
 
Hey, great you got Civ4lerts working that quickly! Looking forward for more contributions.

That would also work, but it is a fragile solution. Whenever you want to add a new method to BugEventManager, you would have to inspect CvEventManager to make sure you are not overriding something by mistake.

Hmm, I don't think so. There is really not much chance that anyone would want to add something to BugEventManager (Caveman2Cosmos added a new event to it IIRC, but that of course wouldn't conflict with the existing EventManager events).

Another method would be letting the overriding functions execute the overridden functions, I'm not sure ATM what the python syntax for that is (probably CvEventManager.onXXXEvent.__get__(self,CvEventManager), as in your fix?). The extra event handler additions would be omitted then.

I think in both cases we have a quick and more or less clean solution; if somebody wants to add functionality to BugEventManager, he should just know about inheritance :).

The modularization of the FfH event manager is probably not really required (and since you wouldn't ever want to remove or disable that functionality, the whole "module" approach would be pretty needlessly).

I also looked for all overriding methods in BugEventManager, here's a conclusion:
  • handleEvent(self, argsList) - Obviously intended
  • onGameUpdate(self, argsList) - overrides functionality added in FfH
  • onKbdEvent(self, argsList) - overrides BtS functionality
  • onEndTurnReady(self, argsList) - overrides nothing with nothing; added as "sample event" since it was fixed in BUG (according to doc)

EDIT: Concerning the syntax, BugEventManager.__init__() calls "CvEventManager.CvEventManager.__init__(self)". Why are you using "__get__"?
 
Another method would be letting the overriding functions execute the overridden functions, I'm not sure ATM what the python syntax for that is (probably CvEventManager.onXXXEvent.__get__(self,CvEventManager), as in your fix?). The extra event handler additions would be omitted then.

The syntax for that is much simpler, just CvEventManager.CvEventManager.on???Event(self, args...).

I am not a fan of this approach, because it requires special handling of the overridden events. Also, it still requires changing CvEventManager in order to avoid the double-call issue.

Another solution would be to break out the event handlers (both BTS and FFH) from CvEventManager and put them in their own module.


I think in both cases we have a quick and more or less clean solution; if somebody wants to add functionality to BugEventManager, he should just know about inheritance :).

Honestly, I find BugEventManager to be quite a mess. Even the "documentation" (i.e. the comments at the top) are misleading and sometimes outright wrong.

Barring a complete rewrite of the whole thing, I would rather not muck around with it. That is why my fix was instead focused on insulating CvEventManager from the problems.


The modularization of the FfH event manager is probably not really required (and since you wouldn't ever want to remove or disable that functionality, the whole "module" approach would be pretty needlessly).

No need for that XML enable/disable stuff, you can just add the handlers directly (perhaps best by adding an extra function call in CvEventInterface.py).


EDIT: Concerning the syntax, BugEventManager.__init__() calls "CvEventManager.CvEventManager.__init__(self)". Why are you using "__get__"?

Because I needed to bind the methods to the object instance without actually calling them. Python has no explicit syntax for that when the methods are not part of the same class as the object.
 
I am not a fan of this approach, because it requires special handling of the overridden events.
Well, I guess that's a matter of opinion; I'd not consider calling overridden functions being something special, having learned OO with Java.

Also, it still requires changing CvEventManager in order to avoid the double-call issue.
Why? Sorry if I'm a bit slow on the uptake, but I think you'd just have to remove the lines in BugEventManager that are adding the handlers a second time.

Honestly, I find BugEventManager to be quite a mess. Even the "documentation" (i.e. the comments at the top) are misleading and sometimes outright wrong.
Yeah, may be.
I remember I got another issue in BUG or a mod based on it where a certain module loading function catches all python exception, instead of just the ones they wanted to catch; in the end, even a syntax error I made was resulting in a totally wrong error message. It was seemingly the result of over-complexity; the code that was causing the issue was really hard to understand out of context.
So, again, if you are interested in working on fixes for BUG, you should contact other people in this community. BUG itself is no longer in development, but many big mods based on BUG.

Barring a complete rewrite of the whole thing, I would rather not muck around with it. That is why my fix was instead focused on insulating CvEventManager from the problems.
Yeah, that's a good point.

No need for that XML enable/disable stuff, you can just add the handlers directly (perhaps best by adding an extra function call in CvEventInterface.py).
You are not speaking of a BUG-style modular event manager?
Because I needed to bind the methods to the object instance without actually calling them. Python has no explicit syntax for that when the methods are not part of the same class as the object.
Ah, thanks; my bad, an error in reasoning.
 
To get it (and other things) working in single-player and non-networked multi-player, you need to undo one of Kael's old changes. When he added the OOS-logger, he also restricted the gameUpdate event to only fire in networked multi-player games. I do not know why he did that, since the only code that was affected at the time was the OOS-logger, which only activates when the game goes out of sync (which of course can only happen in networked multi-player games).

Excellent! Thanks for the fix! It will be included in my next release.

Next, you need to actually enable Civ4lerts

The user can enable or disable Civ4lerts as they like via the BUG options interface in-game.
 
Well, I guess that's a matter of opinion; I'd not consider calling overridden functions being something special, having learned OO with Java.

I meant that it requires treating some event handlers differently than others, i.e. you have to add special code just for a few handlers, and then - in the future - try to remember why those are so special.


Why? Sorry if I'm a bit slow on the uptake, but I think you'd just have to remove the lines in BugEventManager that are adding the handlers a second time.

The "second time" is when the overriding handler is added. The overridden handler is added along with all the others in a simple loop. You would need to either change CvEventManager or add special code to explicitly exclude certain event handlers.

Doing it the other way (removing the "second time") would mean that the code now relies on a bug in order to work.


You are not speaking of a BUG-style modular event manager?

You can just put a function somewhere with a bunch of calls to BugEventManager.addHandler in it, and then make sure that function gets called at some point after the event manager has been initialized.
 
I meant that it requires treating some event handlers differently than others, i.e. you have to add special code just for a few handlers, and then - in the future - try to remember why those are so special.
Well, you would only treat handlers for events already in BtS one way, and handlers added in BUG the other way... but as I said, that's coding style and a matter of opinion.
The "second time" is when the overriding handler is added. The overridden handler is added along with all the others in a simple loop. You would need to either change CvEventManager or add special code to explicitly exclude certain event handlers.

Doing it the other way (removing the "second time") would mean that the code now relies on a bug in order to work.
Relies on a bug? Yes, the CvEventManager handler would no longer be called directly, but indirectly (through the BugEventManager handler). Do you consider that a bug? I'd call that proper use of inheritance :).

And, for that we don't misunderstand each other, I'm talking about modifying the original BugEventManager without your fix; and when I was talking about "second time", I meant these lines in BugEventManger.__init__():
Code:
self.addEventHandler("kbdEvent", self.onKbdEvent)

self.addEventHandler("gameUpdate", self.onGameUpdate)

You can just put a function somewhere with a bunch of calls to BugEventManager.addHandler in it, and then make sure that function gets called at some point after the event manager has been initialized.
OK, that's probably less error-prone; though not fully "modular" IIRC (since you need to call the initialization function somewere).

I think we got in sort of a coding style discussion, and since at the moment I'm not really interested in working on this, I should probably not whine. If you want to fix that, do it your way; furthermore, I'd consider your "dirty" fix reasonable also for long-term, I was just thinking of an quicker/easier way to do it.
 
The user can enable or disable Civ4lerts as they like via the BUG options interface in-game.

Ah, I did not realize you had integrated the BUG options screen as well.

As an aside, I think I know why the pre-chop option does not work (bug #155). I need to do some testing to be sure; if it works out, I will post the solution.



but as I said, that's coding style and a matter of opinion.

Agreed.


And, for that we don't misunderstand each other

Yeah, we are talking about the same thing. Personally, I do not like that solution primarily because it requires writing the event handlers very differently depending on whether there is an existing handler in CvEventManager or not. I prefer to avoid special-casing whenever possible.


If you want to fix that, do it your way; furthermore, I'd consider your "dirty" fix reasonable also for long-term, I was just thinking of an quicker/easier way to do it.

Unless I get a sudden urge to rewrite the whole thing, I do not think I will be messing around any more with the event managers.

Ultimately, any of the solutions mentioned in this thread would work, and Tholal can choose to handle it any way he sees fit.
 
Top Bottom