Pickling Error

[Baldyr]

I'm getting this exception where the game is complaining about not being able to import a pickle module:

Spoiler :

Traceback (most recent call last):
File "CvEventInterface", line 23, in onEvent
File "CvRFCEventManager", line 108, in handleEvent
File "CvRFCEventManager", line 119, in _handleDefaultEvent
File "CvRFCEventHandler", line 233, in onPreSave
File "PyScenario", line 49, in save
cPickle
.
PicklingError
:
Can't pickle <type 'Boost.Python.instance'>: import of module Boost.Python failed

ERR: Python function onEvent failed, module CvEventInterface

This is the save() method from PyScenario (line #49 in red):

from CvPythonExtensions import *
import CvUtil
...
import cPickle as pickle
...
gc = CyGlobalContext()
...

class Setup:

        def __init__(self):
                self.triggers = []
                ...

        ...

        def save(self):
                scriptDict = pickle.loads( gc.getGame().getScriptData() )
                scriptDict['lTriggers'] = self.triggers
                [COLOR="Red"]gc.getGame().setScriptData( pickle.dumps(scriptDict) )[/COLOR]

Line #49 in red. The self.triggers field contains a list of instances of a class ("Trigger"), by the way. (The rest of the dictionary contains stuff related to RFC mod that I use as a base for my own work.)

The exception occurs both when I start up a new game and after I make changes to the .py files during game. The reason for pickling the triggers in the first place is because I don't want them to reload every time the Python files are initialized. So I thought I'd store then in scriptDict so that this method can unpickle them on load:

        def load(self):
                scriptDict = pickle.loads( gc.getGame().getScriptData() )
                self.triggers = scriptDict['lTriggers']

The weird thing is that it worked yesterday, but I've been fiddling with the CvRFCEventHandler today and now I get this exception. Any ideas what causes it?

Also, this is the call from the handler:

from PyScenario import Setup
PyScenario = Setup()
...
        def __init__(self, eventManager):
                ...
                eventManager.addEventHandler("OnPreSave",self.onPreSave)
                ...

        def onPreSave(self, argsList):
                PyScenario.save()

Any help would be appreciated, as I haven't done any pickling before and really have no clue as to what is happening...

 

[Baldyr]

Looking at the Debug Log, the first time the save() method is called by onPreSave() this happens:

PY:OnPreSave

load_module pickle

Subsequent invocations of the method will however result in this:

PY:OnPreSave

load_module Boost

Boost

import failed

This tells me that the game is importing the wrong module and that "pickle" is associated with the wrong module. But the PyScenario module clearly states (in __main__):

import cPickle as pickle

Perhaps the debug message should in fact say:

load_module cPickle

 

[Baldyr]

Ok, this much I've gathered: The problem resonates from the fact that I'm storing objects in the list variable (type "Boost.Python.instance"). That is why the game is trying to access the Boost.Python module. Why this isn't working I have no clue, and also why it figures it can just use cPickle on the first invocation is also a mystery.

Perhaps I need to, somehow, reiterate that it is still a dictionary I'm pickling and not any "Boost.Python.instance"? Or maybe I shouldn't be using the getScriptData() and setScriptData() methods at all, but pickle my list of objects in another way?

 

[Baldyr]

I'm still banging my head against the wall - I tried using the SD Toolkit and after jumping through some hoops it gave me the exact same result. So no luck yet.

I did, by the way, forgot to mention that while the self.triggers list holds instances of the Trigger class, each Trigger object also holds instances of the classes Conditions and Actions. So there would be nested objects in the objects, so to speak. Maybe this is the cause of the exception?

The official Python Tutorial talks about the __getstate__ and __setstate__ methods but I'm not quite following how this would work - and I have no idea if this applies to pickling objects for storage with setScriptData(). I think I would also have to define these methods for the subclasses, so in that case I might just skip those entirely and store everything in the Trigger objects themselves.

And I thought I was done with this yesterday!

 

[EmperorFool]

You cannot pickle game (CvPythonExtension) objects such as CyUnit, CyCity, CyMap, etc.--anything that points to an object from the DLL. You can pickle regular Python objects like list, dict, Trigger (as long as it doesn't contain game objects). As Pickle will store the entire graph of objects you need to ensure nothing points to something that points to something . . . that points to a game object.

I would bet that you can pickle things like CombatTypes and DomainTypes, but try it out before you write a bunch of code that depends on it.

This restriction is not solvable by finding some magical Boost.Python module that it can import. Those objects have memory addresses in them that will not be where those objects live when the data is unpickled. So if you were able to pickle them, unpickling would result in invalid pointers to some random memory location.

Instead of pickling a CyCity, pickle the owner's and its ID as a tuple:

cityId = (pCity.getOwner(), pCity.getID())

 

[Baldyr]

Ok, thanks for the explanation. I haven't checked if it will work yet, but what you said makes a whole lot sense and I also figured out where I'm referencing those Cy objects. I'll get rid of those and it should work. This actually also explains why it worked yesterday and not today, because I wasn't testing one particular feature then.

Thank you one more time!

edit: I believe I fixed it with some ad hoc-code. Instead of carrying over references to Cy objects these fields are now set to None before the code exits. That way there should be no more of this nonsense once the call is made from onPreSave(). I believe this also fixes other potential issues from old values carrying over to later invocations, so this ordeal wasn't for nothing. I learned yet something new about OOP and how fields store values indefinitely - unless they are reset.

 

[EmperorFool]

I learned yet something new about OOP and how fields store values indefinitely - unless they are reset.

This is one of the major benefits of OOP and is how data is encapsulated with the functions that act upon it.

 

[Baldyr]

Yeah, I mostly viewed it as just a handy way of storing values so that they are carried over between methods without having to pass everything as arguments. But I never really reflected over the fact that if some values are being set on one turn, they would remain the same on another turn, should the same trigger fire again. In some cases this would save processing time, in other cases it would lead to an unforeseen outcome. (Like if a plot is cleared for a spawn on one turn, and that spot is automatically chosen another turn, without the check being performed again.)

 

[Baldyr]

Furthermore, I'm still trying to figure out just how CivIV handles pickling. This is pretty much what I've gathered:

In order to get the game to save (and load) user defined values, you store them as strings with setScriptData(). I've also noticed that all the major Cy classes (CyCity, CyPlayer, CyPlot and so on) have this method (or rather, it is inherited from a master class, perhaps CyGlobalContext?).

So you would, for convenience, store all of your values in a dictionary. But before you can store it with CyGame.setScriptData you have to pickle the entire dictionary in order to transform it into a string. So far so good. But what about several strings in one CyGame instance?

Could I have, say, several parallel dictionaries with values (and objects) that are stored with CyGame.setScriptData? Because the way to store and extract the dictionary only seems to use these lines:

dictionary = pickle.loads(game.getScriptData())
...
game.setScriptData(pickle.dumps(dictionary))

And then again:

anotherDictionary = pickle.loads(game.getScriptData())
...
game.setScriptData(pickle.dumps(anotherDictionary))

Wouldn't the second one overwrite the first one?

 

[EmperorFool]

If you recall from the code I had written up back when we started this conversation there was a class called Context in one of my posts. An instance of this was created at the start of processing the triggers and passed among them. This context held values pertinent to the turn/trigger/action/whatever much as you are storing values in the trigger itself.

By using a separate class you can throw away all those values at the end of processing. This is another common paradigm in computing.

 

[EmperorFool]

Each object can hold exactly one string, so yes the second setScriptData() call overwrites the first. One solution is to create a single object containing all those dictionaries and pickle that. In BUG I originally used Stone-D's Toolkit, and for 4.3 I wrote a wrapper around it to maintain compatibility and provide a nicer interface.

SdToolkit maintains a dictionary of dictionaries. The root dictionary stores the other dictionaries as pickled data. The advantage is that unpickling a single dictionary only requires two dictionaries to be unpickled rather than all dictionaries on the (probably correct) assumption that unpickling a pickled pickled string is very fast.

You can also store objects in all of the other Cy objects if you like. SdToolkit advanced allows this, and there are probably some good cases for it. But for a generic data storage interface I chose to stick with using CyGame exclusively.

 

[Baldyr]

If you recall from the code I had written up back when we started this conversation there was a class called Context in one of my posts. An instance of this was created at the start of processing the triggers and passed among them. This context held values pertinent to the turn/trigger/action/whatever much as you are storing values in the trigger itself.

By using a separate class you can throw away all those values at the end of processing. This is another common paradigm in computing.

I'm looking back at this and realize it was something I never really grasped and thus have not implemented in my own code...

I think I will try something similar though, maybe a generic Context object that stores all the relevant values. Right now I believe that I'm passing on the argsList from the EventHandler to the Conditions along with a reference to a Key object containing some additional values. The Actions also get the Key object reference.

This requires some rewrites, unfortunately. I think I'll just take away the entire Key thing for now and build in some sort of Context instead. Then I'll figure out how to make the Key feature fit into the grand scheme of things.

 

[Baldyr]

So if I'm understanding the computer science of this correctly, I would store some values (player ID, int coords) in the Trigger instance when the trigger is created (since these would be global values for all the conditions and actions). Then, on each call from the Event Handler, the code would call the constructor of the Context class and pass the Trigger instance as an argument. This creates a temporary Context object that has the values stored in the Trigger object as default fields on init (and other values set to "None" as default).

This Context object would then be passed on to each Condition as they are checked, as well as to each Action as they are fired. So instead of going "if self.player..." the code would work with "if pContext.player...". It would all basically work the same as now, but once the trigger is done (for this occasion) there would be no trace of the Context object left.

That would make it hazard-free to store Cy objects as Context fields, also.

I could of course make the Context object a field of the Condition and Action objects, but then I'd have to remove it once each is processed. But then again, I wouldn't have to pass along pContext as an argument to every single method involved. So I could just go "if self.context.player..." instead.

The way I solved my pickling issue with the now defunct Key class was that I called Key.__init__() after the processConditions() and initActions() invocations respectively. Because this set everything back to the default None-state.

But with the self.context field it would probably just be easier to go "self.context = None". And then a new Context object would be slapped on there next time around.

Any thoughts?

 

[EmperorFool]

You understand the basics here. I still recommend against storing the Context object anywhere as it belongs to "the current check of all triggers". Think about responsibilities. The reason the Trigger doesn't hold the current turn and current player is because the Trigger doesn't own them. The Trigger owns the player # that it should trigger for, and the coordinates where it should trigger. It does not own the current game turn, etc.

First, I would create the Context once before checking all the triggers. It should hold values and objects about the current state of the game: turn number, player's turn, CyPlayer of the player, etc. These are things that all Triggers, Conditions, and Actions can share.

If you find there are things that the Conditions create for other Conditions or the Actions you might consider having two contexts. It's really not a lot of work to pass two variables to the check() and perform() functions (or whatever you called them). The nice thing about a Context object is that you can add values without changing function signatures.

One example of this would be an Action that gives a new random unit on a random plot owned by the player. You could make this one action, but then if you want another action that gives a random unit in a city owned by the player you'd have to rewrite a completely new action. Instead you could break this up into three actions: 1. pick a random plot, 2. pick a random unit, 3. give the unit. To support the second feature you'd just add one new tiny action: pick a random city.

All of these actions would store their results in the TriggerContext (local to a single Trigger) instead of the Context (shared among all Triggers). Each Trigger would create its own new empty TriggerContext and pass it along with the Context to all Conditions and Actions, then toss out the TriggerContext at the end.

BTW, if you'd like me to do a quick code review for some pointers or suggestions, ZIP it up and post it or email it to me.

 

[Baldyr]

I've been rewriting the entire code all day and I'll be done with the first revision shortly. Then I can email it to you. (I gather I can find your address in your profile. If not, send it to me in a PM.) The offer is of course too good to pass.

Right now I've only implemented one Context class and that is unique for each trigger when they are checked. Once it is processed, it is discarded. It holds basic values shared by all conditions and actions belonging to that trigger.

The values unique for the Event itself are contained in the argsList that is passed along from the Event Handler. Those are included in the Context object as an args field.

I won't have time to actually test this revision today, but the basic idea should be functional. Once I get this working I'll have to figure out how to implement the Keys functionality. (I commented out it for now.)

 

[EmperorFool]

Can you explain briefly what the keys functionality is supposed to do? What problem does it solve or feature does it add?

 

[Baldyr]

Oh, the Keys are something I came up with some weeks ago, when I was busy doing something else with the code. The idea was that rather than only being able to define a target (tile or map area) it could be neat to be able to define a target by (city) name only (or a combination of name and/or plot and/or player). So the "find()" method was born. Any sequence of strings fed to the method were stored in a Key object when the triggers are initialized at startup. Then they were used for lookups once the triggers are checked.

I also realized that other methods were taking city names as (optional) arguments, so I added those to the list stored in the Key object. So the Trigger.find("Moscow", "Moskva") invocation would add those names to the list of keys. But also the method Trigger.lost("London") would add that name to the keys list, and so on. There would be a Key object and a keys list even without the find() method.

This feature turned out pretty good, actually, once I got it to work the way I wanted. It did get in the way of the pickling issue however, so its disabled for now. I don't foresee any problems with reintroducing it down the line however. But I might get rid of the Key object itself, as it stores Cy objects and pretty much does what the Context object is doing now. Since I will be keeping all the global values in Context, and I might also add the keys list to those. Then its just a matter of hooking the lookup back up and picking up the values (city coordinates, CyPlot, CyCity, city owner and city name).

An email is coming your way, by the way.