Dictionary/Class issue

Xyth · Jul 15, 2012

I'm hoping someone can spot what I'm missing here because I cannot figure out what's going wrong. I've written a function that reads a (fake) XML file and loads the information into a global dictionary. It works fine except for the code in red (right at the bottom):

Spoiler :

Code:

### Imports
from CvPythonExtensions import *



### Globals
gc = CyGlobalContext()
CityDict = {}
PlayerDict = {}



### Classes

class CityInfo:
	def __init__(self):
		self.Civilizations = {}
		self.Leaders = {}
		self.Eras = {}



class CityNameInfo:
	def __init__(self):
		self.CityName = ""
		self.Capital = False
		self.Priority = 0



def readCityListInfos(file):
	'Loads all CityListInfo entries from a file into the city dictionary'
	global CityDict
	for line in file.readlines():

		# Set up city entry and list of names
		if "<CityNames>" in line:
			namelist = []
			city = CityInfo()

		elif "<CityName>" in line:
			namelist.append(getXMLString(line))

		# Parse Civilization info
		elif "<CivilizationType>" in line:
			civ = CityNameInfo()
			civtype = gc.getInfoTypeForString(getXMLString(line))

		elif "<iCivilizationPriority>" in line:
			civ.Priority = int(getXMLString(line))

		elif "</Civilization>" in line:
			city.Civilizations[civtype] = civ

		# Parse Leader info
		elif "<LeaderType>" in line:
			leader = CityNameInfo()
			leadertype = gc.getInfoTypeForString(getXMLString(line))

		elif "<iLeaderPriority>" in line:
			leader.Priority = int(getXMLString(line))

		elif "</Leader>" in line:
			city.Leaders[leadertype] = leader

		# Parse Era info
		elif "<EraType>" in line:
			era = CityNameInfo()
			eratype = gc.getInfoTypeForString(getXMLString(line))

		elif "<iEraPriority>" in line:
			era.Priority = int(getXMLString(line))

		elif "</Era>" in line:
			city.Eras[eratype] = era

		# Add each city entry to dictionary
		elif "</CityListInfo>" in line:
			for iName in range(len(namelist)):
				CityDict[namelist[iName]] = city
			del namelist[:]

			for key, info in CityDict.iteritems():
				iCiv = 9
				print ""
				print "Key:   " + key
				print "Name:  " + CityDict[key].Civilizations[iCiv].CityName
				print "Info:  " + info.Civilizations[iCiv].CityName

				[COLOR="Red"]if info.Civilizations[iCiv].CityName == "":
					CityDict[key].Civilizations[iCiv].CityName = key[/COLOR]
					print "Change: " + CityDict[key].Civilizations[iCiv].CityName

Here's the (fake) XML this function is reading:

Spoiler :

And here's the output from the various print statements:

Spoiler :

One entry from each CityListInfo section (Pylos and Olympia) works as expected; Name and Info are blank and the code in red triggers successfully. But it seems that all the other city entries have this value changed at the same time, which I don't want. If it were working correctly "Key:" and "Change:" should be the same for each city, and "Name:" and "Info:" should be blank.

Any idea why this is happening and what I can do to fix it? This is the first time I've tried to use my own classes, have I set something wrong there perhaps?

(I realize globals are frowned upon but other functions in this module rely on it. And besides, passing CityDict as an argument had the same problem too.)

The_J · Jul 16, 2012

I don't get quite what you're doing and what the variables are supposed to be, so could you explain it a bit more

?

The problem is probably that in Python, an assignment is normally just a pointer to a variable and not a copy of the value. So somewhere something is assigned to multiple things, but I just can't see at the moment where this could be.

God-Emperor · Jul 16, 2012

I expect it is this part:

Code:

			for iName in range(len(namelist)):
				CityDict[namelist[iName]] = city

This assigns every key to point at the exact same city object (for each set - you do create a new object for each different set). It does not copy the data from "city" to a unique instance for each item in the dictionary. There is therefore only one city object and if you change it for one you change it for them all (since there is only one). I'm guessing you want to point each of them at their own copy, instead of pointing them all at the same copy.

In Python everything is a pointer, which may be helpful to know if you know C or C++.

What you want are what is known as "deep copies" since you also want to copy each of the dictionaries that are part of the class, not just copy pointers to the same ones. This will copy all the data.

Generally importing the copy module and using copy.deepcopy(X) will get you a copy of everything. It usually works. If it doesn't, you need to define a copy function for the class that does the job. So, assuming you have an "import copy" at the top of the file, you'd just change that chunk of code to be:

Code:

			for iName in range(len(namelist)):
				CityDict[namelist[iName]] = copy.deepcopy(city)

Then each thing in the dictionary will have its own copy of everything.

This may well copy a lot of data that doesn't really need to have a bunch of copies around, but that is a whole different issue involving how things are designed.

Xyth · Jul 16, 2012

The_J said:
The problem is probably that in Python, an assignment is normally just a pointer to a variable and not a copy of the value. So somewhere something is assigned to multiple things, but I just can't see at the moment where this could be.

God-Emperor said:
I expect it is this part:

Code:

for iName in range(len(namelist)): CityDict[namelist[iName]] = city

This assigns every key to point at the exact same city object (for each set - you do create a new object for each different set). It does not copy the data from "city" to a unique instance for each item in the dictionary. There is therefore only one city object and if you change it for one you change it for them all (since there is only one). I'm guessing you want to point each of them at their own copy, instead of pointing them all at the same copy.

In Python everything is a pointer, which may be helpful to know if you know C or C++.

What you want are what is known as "deep copies" since you also want to copy each of the dictionaries that are part of the class, not just copy pointers to the same ones. This will copy all the data.

Generally importing the copy module and using copy.deepcopy(X) will get you a copy of everything. It usually works. If it doesn't, you need to define a copy function for the class that does the job. So, assuming you have an "import copy" at the top of the file, you'd just change that chunk of code to be:

Code:

for iName in range(len(namelist)): CityDict[namelist[iName]] = copy.deepcopy(city)

Then each thing in the dictionary will have its own copy of everything.

Ahh, yes, that'll be it. I wasn't aware assignments were pointers in Python, that explains many things (and makes some things much easier to code). Makes me wonder if I shouldn't convert the CityNameInfo class into a tuple, since once it's loaded into the CityDict it doesn't need to change in either dictionary once in.

The_J said:
I don't get quite what you're doing and what the variables are supposed to be, so could you explain it a bit more ?

God-Emperor said:
This may well copy a lot of data that doesn't really need to have a bunch of copies around, but that is a whole different issue involving how things are designed.

I'm creating a module that will select city names dynamically, rather than sequentially like in BTS. Cities can be assigned priority (or even entirely different names) for different civilizations, leaders and eras, and whether they are a suitable capital or not. I may add other factors in future.

The fake xml I quoted above is actually one of a pair. That one lets you enter a group of citynames with the same settings, the other one lets you enter one city at a time but with more options available. They both load into the same dictionary though. Here's a couple of examples from the other fake xml file, it might give a better idea of why I've set the class structures up the way I have:

Spoiler :

Basically what I'm doing is creating a master dictionary at game start/load of all city entries (the CityDict), loaded from the two fake xml files. After that I create a second dictionary (the PlayerDict) that contains pointers to the appropriate leader entry (first priority) or civilization entry. If neither exists, then that city isn't eligible for that player.

When a city is founded a function is called that chooses a high priority city name for that player, adjusted for era. City is renamed and that city is removed from the PlayerDict for all players so it can't be founded again.

If you guys have any suggestions on how to do this more efficiently or elegantly, I'm always open to learning how. This is the first time I've attempted a project of this magnitude.

The_J · Jul 16, 2012

Sounds interesting

.
You should definitely think about releasing this as a mod component :mischief:

.

I'm not sure if I'd choose a structure like you have there, because there's lot of redundancy and it makes adding names a more tiresome process (unless I'm missing something).
Instead of the priority tag, I'd go with a basic list at the beginning, and priority would be the order of the list. And a separate list for possible capitals.
But I guess you had some thoughts when you created your system, which I don't see, so it's probably exactly what you need and doesn't need any changes

.

Xyth · Jul 16, 2012

The_J said:
Sounds interesting .
You should definitely think about releasing this as a mod component .

Yep, I plan to. I won't be converting the BTS citylists or anything, but I'll provide some instructions and useful examples and modders can then add new entries to suit their needs.

I'm not sure if I'd choose a structure like you have there, because there's lot of redundancy and it makes adding names a more tiresome process (unless I'm missing something).

Something I should have mentioned is that if there are no eligible cities for a player, the system will fall back on the citylist defined in CIV4CivilizationsInfo.xml. Because of this, you can enter as few or as many entries as you wish and you don't have to have entries for every civilization.

So for example, you could set Perikles' capital as Athens, Alexander's capital as Pella, and then let all other cities be founded in order. Or define a single city like Tsaritsyn to be founded by that name for Peter and Catherine but as Stalingrad by Stalin. And have it named Volgograd if founded in the modern era.

I'm trying to make the system flexible and incremental to add to.

Instead of the priority tag, I'd go with a basic list at the beginning, and priority would be the order of the list. And a separate list for possible capitals.
But I guess you had some thoughts when you created your system, which I don't see, so it's probably exactly what you need and doesn't need any changes .

I originally tried a priority ordered list for the PlayerDict but I had some trouble with it. I may take another attempt now that I'm aware of my misunderstanding about assignments.

The_J · Jul 17, 2012

Xyth said:
Something I should have mentioned is that if there are no eligible cities for a player, the system will fall back on the citylist defined in CIV4CivilizationsInfo.xml. Because of this, you can enter as few or as many entries as you wish and you don't have to have entries for every civilization.

Ah, that's good, makes the thing easier.

The whole thing is definitely an interesting idea :thumbsup:

.

Dictionary/Class issue

Xyth

History Rewritten

The_J

Say No 2 Net Validations

God-Emperor

Deity

Xyth

History Rewritten

The_J

Say No 2 Net Validations

Xyth

History Rewritten

The_J

Say No 2 Net Validations

Similar threads