James the Wise
Chieftain
- Joined
- Nov 20, 2001
- Messages
- 62
These OOS dont seem to be on the End or start of a turn but just randomly during a players turn
Someone (Tasunke, maybe?) said that differing BUG options can sometimes cause OOS issues.
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?
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?
## EVENTLIST
self.EventHandlerMap = {
'mouseEvent' : self.onMouseEvent,
'mouseEvent' : CvEventManager.onMouseEvent.__get__(self,CvEventManager),
Thanks for looking into that and solving that issue, but I think your solution is not perfect, because you misunderstood some things.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:
Then do the same for the next 80 or so similar lines. It really helps if you use a regex-capable search & replace tool.Code:'mouseEvent' : CvEventManager.onMouseEvent.__get__(self,CvEventManager),
The next part is not strictly necessary; just a little future-proofing:
Change all occurences of "self.beginEvent(" with "CvEventManager.beginEvent(self,".
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
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:
The code commented with #FfH... was obviously added in FfH. So in BtS the function did exactly nothing, and BUG could override it.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
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.
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.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.
You mean f.e. renaming the methods BugEventManager overrides in BugEventManager itself so it no longer overrides the ones in CvEventManager?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.
Yeah, I understand now, the problem seems to lie in BugEventManagerThat would be one way to go, but the fix is still necessary to avoid some handlers being called twice for each event.
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__()?
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.
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!
//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
<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."/>
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.
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).
EDIT: Concerning the syntax, BugEventManager.__init__() calls "CvEventManager.CvEventManager.__init__(self)". Why are you using "__get__"?
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 am not a fan of this approach, because it requires special handling of the overridden events.
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.Also, it still requires changing CvEventManager in order to avoid the double-call issue.
Yeah, may be.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, that's a good point.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.
You are not speaking of a BUG-style modular event manager?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).
Ah, thanks; my bad, an error in reasoning.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.
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).
Next, you need to actually enable Civ4lerts
Well, I guess that's a matter of opinion; I'd not consider calling overridden functions being something special, having learned OO with Java.
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.
You are not speaking of a BUG-style modular event manager?
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.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.
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 .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.
self.addEventHandler("kbdEvent", self.onKbdEvent)
self.addEventHandler("gameUpdate", self.onGameUpdate)
OK, that's probably less error-prone; though not fully "modular" IIRC (since you need to call the initialization function somewere).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.
The user can enable or disable Civ4lerts as they like via the BUG options interface in-game.
but as I said, that's coding style and a matter of opinion.
And, for that we don't misunderstand each other
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.