(Newbie Warning) Coding Rogue-Clone in Python

A place to discuss the implementation and style of computer programs.

Moderators: phlip, Moderators General, Prelates

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Ben-oni » Mon May 21, 2012 6:49 pm UTC

Try this:
Code: Select all
xyzs = [key for key, value in grid.iteritems() if value == self]
xyz = xyzs[0]

You might want to consider keeping the coordinates within the cell anyways, though. This requires you to go through the entire grid to find the location of a single element.
Ben-oni
 
Posts: 268
Joined: Mon Sep 26, 2011 4:56 am UTC

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Mon May 21, 2012 6:50 pm UTC

Zamfir wrote:A dictionary is a lookup table, or hash table. It is optimized to very quickly find the value that belongs to a specific key (wikipedia will explain how). Every key must be unique, but the same value can be used for multiple keys.
In this case, every key has an individual cell object (if two cell objects repeated, that'd actually be a very bad thing--two cells occupying the same coordinate!). Still, point taken.
Zamfir wrote:Why can't you just remember the key, get the cell from the dictionary, and send (key, Cell) to wherever you want?
It's the draw function for the Cell object, and...

Oh, for crap's sake. Yeah, I'm rendering the screen by doing a 'for key in grid:' loop (grid is the single z-level dictionary) and rendering every cell those keys hold. I can just pass the key along to the Cell's draw function every time I call it. Didn't even occur to me.

At least I know (vaguely!) how generators work, now.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Zamfir » Mon May 21, 2012 7:04 pm UTC

The Great Hippo wrote:In this case, every key has an individual cell object (if two cell objects repeated, that'd actually be a very bad thing--two cells occupying the same coordinate!). Still, point taken.

Yeah, but the dictionary doesn't know...

If you really need to do lookups in both directions, and you know that the mapping is one-on-one, you can just use 2 dictionaries. Though I think python doesn't like you to use objects as keys

In this case, it seems defensible to store the x and y value of a cell in the cell object, if that is turns out handy. As long as you are sure that you willl never move cells, because that would give you a world of hurt to keep their internal x,y,z synchronized to the lookup table.
User avatar
Zamfir
 
Posts: 5745
Joined: Wed Aug 27, 2008 2:43 pm UTC
Location: Nederland

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby EvanED » Mon May 21, 2012 7:20 pm UTC

Zamfir wrote:But doing this is a sign that your program is not well-thought out enough.

I haven't read much of this thread, but I think that's unfair. There are a lot of times when you need a bidirectional lookup. In fact, Boost has a whole library, bimap, dedicated to providing that "one" piece of functionality. The real problem is that Python doesn't seem to have a standard bidirectional map structure.

A "normal" way of dealing with this would be to keep two dictionaries, one forwards and one backwards, but then you have to keep them in sync and blah blah blah. It's totally reasonable to think that, at least until it becomes an actually-measurable performance hit, doing the reverse conversion by a linear scan occasionally is preferable.

Edit: oh, another page

Zamfir wrote:Though I think python doesn't like you to use objects as keys

You can, you just have to define a __hash__ function, then make sure the hash can't change. (Or, because this is Python, "make sure the hash can't change via means other than being a dick", because making sure it can't change is nigh impossible.) In general that means also overriding the __setattr__ function to turn it into a no-op. (You can also let through changes to attributes that __hash__ doesn't look at.)

The point behind that requirement is because if the hash of a key changes, that screws up an invariant of the hash table, and it will have trouble finding that object again in the future. To wit:
Code: Select all
$ python3
Python 3.2 (r32:88445, May  3 2011, 13:26:55)
[GCC 4.1.2 20080704 (Red Hat 4.1.2-50)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> class C:
...     def __hash__(self):
...             return hash(self.x)
...     def __init__(self):
...             self.x = 5
...
>>> c = C()
>>> c.x
5
>>> d = {c : "hi"}
>>> d[c]
'hi'
>>> c.x=10
>>> d[c]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: <__main__.C object at 0xb4d0a10>
Last edited by EvanED on Mon May 21, 2012 7:26 pm UTC, edited 1 time in total.
EvanED
 
Posts: 3767
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Mon May 21, 2012 7:21 pm UTC

Now that I have the rendering problem fixed, I can't think of any reason why I'd need the x-y-z coordinate of a cell independent of its key in the dictionary (in which case, I have the x-y-z coordinate; it's the key I used to get the cell). But it might pay dividends to create a reversed dictionary just in case (which would actually probably be pretty easy; I'd just set the reverse dictionary to 'update' every time I ran the function that separates the grid from the cube--since that only happens when a cell is created or destroyed, or you change z-levels). I'll probably end up doing this if a good reason comes up for it in the future.

The big problem I'm encountering now is a persistent bug that's been refusing to go away ever since I introduced Cell inventories a few hours ago. When the cell's draw function is called, it first draws itself (its background color), then it goes through its inventory and draws any objects present there. But when I set an object down in a cell's inventory, the object appears in every cell's inventory, leading to the same instance of an object appearing in every possible cell that exists. Here's the relevant code (it's pretty raw, as I'm just trying to prove that it's doable):
Code: Select all
class Thing(Object):
   def __init__ (self, properties = {}, container = []):
      self.properties = properties
      self.container = container

#for now, BASE_THING is our only thing
def createThing(prop = BASE_THING, cont = []):
   return Thing(prop, cont)

#gets relevant Cell object
cell = cube[(x, y, z)]
cell.properties['inventory'].append(createThing())
I've tested this over and over again; cube[(x, y, z)] definitely returns the relevant cell object (there's no problem at all with the cell placement code, or the cell rendering code, both which rely on the Cube() object and its children, and I even added code to print out the object cell was pointing to, and it's always a single instance of a Cell object), but whenever I try to assign a Thing() object to the cell's inventory, it goes to every cell's inventory.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Zamfir » Mon May 21, 2012 7:31 pm UTC

Can you show the code that defines the Cell class? Adn that instantiates the Cells
Last edited by Zamfir on Mon May 21, 2012 7:36 pm UTC, edited 1 time in total.
User avatar
Zamfir
 
Posts: 5745
Joined: Wed Aug 27, 2008 2:43 pm UTC
Location: Nederland

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Mon May 21, 2012 7:36 pm UTC

Code: Select all
class Cube(dict):
   def setGrid(self, z):
      keys = cube.keys()
      keys = filter(lambda keys: keys[2] == 0, cube.keys())
      for i in keys:
         grid[i] = cube[i]

class Cell(Object):
   def __init__ (self, properties = {}, container = []):
      self.properties = properties
      self.container = container
   def draw(self, xyz):
      x, y = xyz[0], xyz[1]
      putBack(mainmap, x, y, self.properties['color'])
      for thing in self.properties['inventory']:
         putChar(mainmap, x, y, thing.properties['color'], thing.properties['char'])
      if self.properties['inventory'] == []:
         putChar(mainmap, x, y, libtcod.white, " ")
Here's both the Cell and Cube class. If it helps, cube is a global object that starts as Cube(), and one of the first things I do for every test is assign cube a bunch of tuple keys linked to instances of the Cell object.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Zamfir » Mon May 21, 2012 7:37 pm UTC

Adn that instantiates the Cells?
User avatar
Zamfir
 
Posts: 5745
Joined: Wed Aug 27, 2008 2:43 pm UTC
Location: Nederland

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Mon May 21, 2012 7:39 pm UTC

Oops, pardon!
Code: Select all
#for now, 'BASE_CELL' are the default properties of a cell.
def createCell(prop = BASE_CELL, cont = []):
   return Cell(prop, cont)

cube[(x, y, z)] = createCell()
#we just created a new cell, so we need to update the grid
cube.setGrid(z)
renderAll()
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Zamfir » Mon May 21, 2012 7:44 pm UTC

with this code, the 'properties' property of ever Cell instance will point to the same object, namely BASE._CELL.

So your append of every cell appends to BASE_CELL

Your intention is probably that 'properties' becomes a shallow copy of the 'properties' attribute of the constructor, not a reference to it
User avatar
Zamfir
 
Posts: 5745
Joined: Wed Aug 27, 2008 2:43 pm UTC
Location: Nederland

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Mon May 21, 2012 7:50 pm UTC

God damn it.

Yep, that fixed it.

I'll look up producing shallow copies next, then!
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Zamfir » Mon May 21, 2012 7:56 pm UTC

a shallow copy of a list is a new list with the same entries as the old list. if the entries are objects, then the new list contains references to the same objects as the old list. In a deep copy, the entries in the list are also copies (which can be deep or shallow in turn.).

In general, shallow copies are OK. But if you find yourself in need of deep copies, you need to think hard if that makes sense. Sometimes it does, usually not

some more small stuff about your code: you do cube.keys twice!

but more important,really: you have named the attribute of the lambda function 'keys', plural. if that was intentional, you are not yet understanding how that filter works.
User avatar
Zamfir
 
Posts: 5745
Joined: Wed Aug 27, 2008 2:43 pm UTC
Location: Nederland

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Mon May 21, 2012 8:04 pm UTC

but more important,really: you have named the attribute of the lambda function 'keys', plural. if that was intentional, you are not yet understanding how that filter works.
Oh, no; pardon! It was originally named 'key', but it was also originally inside of a function (handle_key) with a local variable called 'key' (that stores user-input) while I was testing it, so I changed it to 'keys' to avoid the conflict and never bothered changing it back.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Zamfir » Mon May 21, 2012 8:13 pm UTC

and a final thing, before i go to sleep. Why do your objects have a 'properties' field? It's like having a suitcase inside your suitcase. Yo dog, we heard you like properties so we put properties in your properties.

Technically, a python object already is a special kind of dictionary, and its field names are its keys.
When you say
myInstance.niceField =5

you're just using a nicer syntax for almost the same action as
myDict['niceKey'] = 5
User avatar
Zamfir
 
Posts: 5745
Joined: Wed Aug 27, 2008 2:43 pm UTC
Location: Nederland

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Mon May 21, 2012 8:28 pm UTC

Hm. I'm going to have to read up more on that; could I just replace all the object.properties['x'] references with object['x'] references, for example? Would I just drop the 'container' into a key inside of an object? Is that something I declare at the initialization of a class, or can I just set the keys whenever?
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby EvanED » Mon May 21, 2012 8:36 pm UTC

The Great Hippo wrote:Hm. I'm going to have to read up more on that; could I just replace all the object.properties['x'] references with object['x'] references, for example?

It'd be replaced with object.x. (You could make it do object.['x'] if you want by defining the appropriate function, maybe __setitem__ and __getitem__.)

This does illustrate two advantages the dictionary-like syntax has: (1) the keys don't have to be identifiers and (2) you can access them with a key which isn't a literal. (You can't, of course, say x = "foo"; print obj.x and expect that to act like print obj.foo. You have to write the ugly print obj.__getattr__(x).)

So there are three choices:
1) Leave it as is, with object.properties['x']
2) Use attributes of the object instead of a dict, giving object.x
3) Make the object look like a dictionary, giving object['x']

Each has its advantages and disadvantages.
EvanED
 
Posts: 3767
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Mon May 21, 2012 8:44 pm UTC

Actually, I remember now why I went this route a long time ago, although I don't know if my reasons are still valid:

I wanted to have crap-tons of properties, leaving it open so I could add properties willy-nilly. I was concerned that if I defined all the properties as attributes, I'd end up having a massive and complex list of properties, many of which wouldn't even see use (a wall, for example, probably has no need for hit points). Assigning properties as a dictionary seemed like the best option--I get to define what I want and have objects with very specialized properties.

Now that I understand dictionary is a class, though, I could always make the relevant objects inherit the dictionary class... but that sounds a little dangerous.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby EvanED » Mon May 21, 2012 8:54 pm UTC

The Great Hippo wrote:I wanted to have crap-tons of properties, leaving it open so I could add properties willy-nilly. I was concerned that if I defined all the properties as attributes, I'd end up having a massive and complex list of properties, many of which wouldn't even see use (a wall, for example, probably has no need for hit points). Assigning properties as a dictionary seemed like the best option--I get to define what I want and have objects with very specialized properties.

You could still do that if you use attributes... just don't give the wall a .hit_points attribute. :-)
EvanED
 
Posts: 3767
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Mon May 21, 2012 9:00 pm UTC

EvanED wrote:You could still do that if you use attributes... just don't give the wall a .hit_points attribute. :-)
You can do that? I assumed that objects from the same class share the same attributes; there isn't a way to be selective about your attributes beyond creating a new class (or overwriting the old class with a child class that has its own __init__ constructor). Either way, I can imagine the list of defined attributes would get... unusually long--and pretty messy looking.

EDIT: One more thing I'm working on figuring out--when is it a good time to insert exception code and when isn't it? And creating user-defined exceptions--this is something I'm still trying to grasp. Here's a relevant example:
Code: Select all
class Error(Exception):
   pass


class System(Object):
   def __init__ (self, properties = {}):
      self.properties = properties

   def draw(self):
      if hasPropVal(self, 'x') and hasPropVal(self, 'y') and hasPropVal(self, 'z'):
         if not hasPropVal(self, 'color'):
            raise Error("system object " + str(self) + " has no color property")
         if not hasPropVal(self, 'char'):
            raise Error("system object " + str(self) + " has no char property")
         try:
            putChar(mainmap, self.properties['x'], self.properties['y'], self.properties['color'], self.properties['char'])
         except:
            raise Error("system object " + str(self) + " has an invalid property")
      else:
         raise Error("system object " + str(self) + " has no x-y-z coordinate")
The above is a lot of error-code, and I'm not sure how necessary all of it is; a system object is something like a cursor, or a highlighted cell (so it functions independent of game-logic concerning where you can and can't move). If an object like a cursor doesn't have a color, should I raise an error--or should I just 'return' and cause the system to not draw the cursor? I can think of cases where the latter would be a better move(I could use the draw function as a sort of test, for instance--to draw only the system objects with a set color, or a set character), but most of the time, I imagine it would be better to actually raise an error. If that's the case, is the above an example of moving in the right direction as far as exception coding goes?
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby EvanED » Mon May 21, 2012 9:38 pm UTC

The Great Hippo wrote:
EvanED wrote:You could still do that if you use attributes... just don't give the wall a .hit_points attribute. :-)
You can do that? I assumed that objects from the same class share the same attributes; there isn't a way to be selective about your attributes beyond creating a new class (or overwriting the old class with a child class that has its own __init__ constructor).

This has an inkling of truth, in that if __init__ sets the .hitPoints property, then it'll be set. :-)

But I suspect a key point you may be missing is that the attributes that get set by the constructor aren't closed... you can add new ones later. E.g.:
Code: Select all
class C:
    def __init__(self):
        self.x = 5

c = C()
c.x         # ==> 5
c.y = 7
c.y         # ==> 7


So it really is like the case of keys in the properties dict. After all, if you set self.properties['whatever'] in __init__, then that property would be shared among all the instances too. :-)

As for your error question, my guideline is "if you have enough information to decide what to do, then decide then; otherwise propagate it up." E.g. if you're trying to decide whether the draw() function should become a no-op if there's no color or if it should throw an error, then it should throw, because someone who called draw() can catch the error. But eventually you get to a layer where you say "now I'm going to try to draw this thing, but it may not succeed" and that's where you should catch an error.

(This is guided by the observation that a lot of time "errors" of that sort are caused by incorrect assumptions you're making about the code, rather than cases where you really want to be able to pass something with no .color attribute and have it do nothing, and that the "slurp the error" behavior is hiding mistakes.)
EvanED
 
Posts: 3767
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Ben-oni » Tue May 22, 2012 12:34 am UTC

You should probably do one or the other of these:
Code: Select all
   def draw(self):
      try:
         putChar(mainmap, self.properties['x'], self.properties['y'], self.properties['color'], self.properties['char'])
      except:
         return   # Do nothing
Code: Select all
   def draw(self):
      putChar(mainmap, self.properties['x'], self.properties['y'], self.properties['color'], self.properties['char'])

Either you don't care about errors, and just fail silently, or let the error trickle up to the calling function. Querying a dictionary for a value it doesn't have just raises an error, so there's no need to manually raise an error if the property is missing.

As for using an object as a shortcut to it's properties, you had good reasons for using a properties dictionary within the object. But a bit of shorthand, as recommended, would be handy:
Code: Select all
class Object:
   def __init__(self, props = {}):
      self.properties = args
      self.container = []
   
   def __setitem__(self, key, value):
      self.property[key] = value
   
   def __getitem__(self, key):
      return self.property[key]
Ben-oni
 
Posts: 268
Joined: Mon Sep 26, 2011 4:56 am UTC

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby EvanED » Tue May 22, 2012 3:10 am UTC

Ben-oni wrote:You should probably do one or the other of these:
Code: Select all
   def draw(self):
      try:
         putChar(mainmap, self.properties['x'], self.properties['y'], self.properties['color'], self.properties['char'])
      except:
         return   # Do nothing

You should virtually never use a plain except in code; put the actual type there. Plain excepts are a huge smell.

Either you don't care about errors, and just fail silently, or let the error trickle up to the calling function. Querying a dictionary for a value it doesn't have just raises an error, so there's no need to manually raise an error if the property is missing.

You may want to translate one error to another, however, for any number of reasons.

That said, "the" Python style is ask for forgiveness, not permission, so you'll see
Code: Select all
try:
    print d['x']
except KeyError:
    print "x not present!"

far more than
Code: Select all
if 'x' in d:
    print d['x']
else:
    print "x not present!"
EvanED
 
Posts: 3767
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Ben-oni » Tue May 22, 2012 3:19 am UTC

EvanED wrote:You should virtually never use a plain except in code; put the actual type there. Plain excepts are a huge smell.

Obviously. I showed error catching earlier; now I'm just showing when.

I think the way I heard it was "It's Easier to Ask for Forgiveness than to Get Permission".
Ben-oni
 
Posts: 268
Joined: Mon Sep 26, 2011 4:56 am UTC

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Tue May 22, 2012 8:30 am UTC

Ben-oni wrote:As for using an object as a shortcut to it's properties, you had good reasons for using a properties dictionary within the object. But a bit of shorthand, as recommended, would be handy:
Code: Select all
class Object:
   def __init__(self, props = {}):
      self.properties = args
      self.container = []
   
   def __setitem__(self, key, value):
      self.property[key] = value
   
   def __getitem__(self, key):
      return self.property[key]
Wait--so can you give me an example of how I'd retrieve a property from this Object class? Say I wanted the list in the 'inventory' property for an object called 'Cell'. Would I type 'Cell['inventory']' with this code? I assume I could assign it the same way?

Part of what's throwing me off is the 'property' versus 'properties' tag. Is property a built-in type? I don't get where or how the class here 'knows' what property means (or does it even care? Is 'property' is just a descriptive name, and you're just telling the code 'hey, store this type of data-structure here'?). I think I need to study classes a bit more--because constructors like these keep throwing me off course.

Speaking of classes and their constructors--updating my error code appropriately. One thing about error types is confusing me: You both mention typed errors (KeyValue versus just Exception, for example); I assume it's a good idea to create user-defined errors, so that's what I'm doing--but beyond creating some classes that inherits the exception type and giving those classes appropriate names (PropertyTypeError, for example, when the code hits a wrong property[key] / value type assignment, like putting an integer where the code expects a string), is there anything else those classes should be doing? Right now, they're just a statement of a new class with exception as its parent, and a pass statement underneath. So, say, PropertyTypeError looks like:
Code: Select all
class PropertyTypeError(Exception):
    pass
And that's all. Is there another element of exception-catching that I'm missing?
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Zamfir » Tue May 22, 2012 10:17 am UTC

Python does a pretty good job of raising understandable exceptions when stuff goes wrong, so you often don't need to raise your own. You usually want the program to stop at such a moment and go look for the bug, so you don't want to catch them either.

So ask yourself: Why do you want to raise a particular exception? Why do you want to catch one? How does it help you? Try to get sharp answers to those questions. If you can't think of a good reason, keep them out.

The 'empty' subclassing is OK, by the way. The main reason you want to do this, is so that catch blocks can recognize your specific exceptions. You raise an exception because you expected something to happen. Then other code somewhere else knows "ah, it failed because of that reason. Then it's OK, I know what to do".

But what if something else went wrong, something you did not expect but caused a python exception?
User avatar
Zamfir
 
Posts: 5745
Joined: Wed Aug 27, 2008 2:43 pm UTC
Location: Nederland

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Tue May 22, 2012 10:51 am UTC

Okay! That helps me understand a bit more about what's going on--I took out most of the error handling so far because most of the critical errors I'm imagining aren't really a big deal yet. Here's the main code, in all its horribly mangled glory:
Spoiler:
Code: Select all
import libtcodpy as libtcod
import copy #'cuz I'll probably need it
import textwrap #'cuz I'll probably need it
from config import * #various variables and stuff

#deals with coordinates--storing and managing them
class Cube(dict):
   def setGrid(self, z):
      libtcod.console_clear(mainmap)
      grid.clear()
      keys = filter(lambda key: key[2] == z, cube.keys())
      for i in keys:
         grid[i] = cube[i]

#parent object of all game objects
class Object():
   pass

#system objects--cursors, selected areas--independent x-y-z
#coordinate system that ignores all game logic
class System(Object):
   def __init__ (self, properties = {}):
      self.properties = properties
   def draw(self):
      putChar(mainmap, self.properties['x'], self.properties['y'], self.properties['color'], self.properties['char'])
      if hasPropVal(self, 'bcolor'):
         putBack(mainmap, self.properties['x'], self.properties['y'], self.properties['bcolor'])

   def move(self, dx, dy, dz):
      self.clear()
      if dz != 0:
         cube.setGrid(self.properties['z'] + dz)
      self.properties['x'] += dx
      self.properties['y'] += dy
      self.properties['z'] += dz
   def clear(self):
      putChar(mainmap, self.properties['x'], self.properties['y'], self.properties['color'], None)
      putBack(mainmap, self.properties['x'], self.properties['y'], None)

#a single tile 'object'
class Cell(Object):
   def __init__ (self, properties = {}, container = []):
      self.properties = properties
      self.container = container
   def draw(self, xyz):
      x, y = xyz[0], xyz[1]
      putBack(mainmap, x, y, self.properties['color'])
      for thing in self.properties['inventory']:
         putChar(mainmap, x, y, thing.properties['color'], thing.properties['char'])
      if self.properties['inventory'] == []:
         putChar(mainmap, x, y, libtcod.white, " ")
   def clear(self, xyz):
      x, y = xyz[0], xyz[1]
      putBack(mainmap, x, y, libtcod.black)
      putChar(mainmap, x, y, libtcod.black, " ")

#a single 'object' (an item, creature, or wall)
class Thing(Object):
   def __init__ (self, properties = {}, container = []):
      self.properties = properties
      self.container = container

class Effect(Object):
   def __init__ (self, name, action = []):
      self.name = name
      self.action = action

#initialization procedures
def start():
   global mainmap, sysmap, cube, grid, cursor, focus
   libtcod.console_set_custom_font('arial10x10.png', libtcod.FONT_TYPE_GREYSCALE | libtcod.FONT_LAYOUT_TCOD)
   libtcod.console_init_root(CON_W, CON_H, 'ROGUE-CLONE ENGINE', False)
   mainmap = libtcod.console_new(MAX_MAP_W, MAX_MAP_H)
   sysmap = libtcod.console_new(MAX_MAP_W, MAX_MAP_H)
   cube = Cube()
   grid = {}
   cursor = System(CURSOR)
   focus = cursor
   renderAll()

def putChar(target, x, y, color, char):
   libtcod.console_set_foreground_color(target, color)
   libtcod.console_put_char(target, x, y, char, libtcod.BKGND_NONE)

def putBack(target, x, y, color):
   libtcod.console_set_back(target, x, y, color)

#checks for presence of obj value, accepts arguments
def hasPropVal (obj, prop, *args):
   if prop not in obj.properties:
      return False
   if len(args) == 0:
      return True
   if len(args) == 1 and callable(args[0]):
      return args[0](obj.properties[prop])
   return obj.properties[prop] in args


#blit contents of consoles onto root console
def renderAll():
   for cell in grid:
      grid[cell].draw(cell)
   focus.draw()
   libtcod.console_blit(mainmap, 0, 0, CON_W, CON_H, 0, 0, 0)

#for now, 'BASE_THING' are the default properties of a thing.
def createThing(prop = {}, cont = []):
   return Thing(prop, cont)

#for now, 'BASE_CELL' are the default properties of a cell.
def createCell(prop = {}, cont = []):
   return Cell(prop, cont)

#get a character key from user key-press
def getKey(key):
   if key.vk == libtcod.KEY_CHAR:
      return chr(key.c)
   else:
      return key.vk

#handle user input
def handle_keys():
   #just to keep things shorter and more concise
   (x, y, z) = (focus.properties['x'], focus.properties['y'], focus.properties['z'])
   key = libtcod.console_wait_for_keypress(True)
   key = getKey(key)

   

   if libtcod.console_is_key_pressed(libtcod.KEY_ESCAPE):
      return 'True'
   if libtcod.console_is_key_pressed(libtcod.KEY_SPACE):
      cube[(x, y, z)] = createCell({'inventory' : [], 'color' : libtcod.grey})
      #we just created a new cell, so we need to update the grid
      cube.setGrid(z)
      renderAll()

   if libtcod.console_is_key_pressed(libtcod.KEY_ENTER):
      cube[(x, y, z)].properties['inventory'].append(createThing({'color' : libtcod.white, 'char' : '@'}))
      renderAll()

   #to test z-level rendering
   if libtcod.console_is_key_pressed(libtcod.KEY_TAB):
      focus.move(0, 0, 1)
      renderAll()

   #to test z-level rendering
   if key == "z":
      focus.move(0, 0, -1)   
      renderAll()

   if key in MOVE_KEYS:
      dx, dy, dz = MOVE_KEYS[key]
      focus.move(dx, dy, dz)
      renderAll()


start()

#main game loop
while not libtcod.console_is_window_closed():

   libtcod.console_flush()
   exit = handle_keys()
   if exit:
      break


And here's the config module, which just contains some constants I've set:
Spoiler:
Code: Select all
import libtcodpy as libtcod

#console width/height
CON_W = 50
CON_H = 50

#max map size
MAX_MAP_H = 50
MAX_MAP_W = 50

#movement keys
MOVE_KEYS = {
   libtcod.KEY_UP : (0, -1, 0),
   libtcod.KEY_DOWN : (0, 1, 0),
   libtcod.KEY_LEFT : (-1, 0, 0),
   libtcod.KEY_RIGHT : (1, 0, 0)
   }

#basic cell properties
BASE_CELL = {
      'floor' : True,
      'color' : libtcod.grey,
      'inventory' : []
   }

#basic thing properties
BASE_THING = {
      'color' : libtcod.white,
      'char' : "@"
   }
#system object(s)

CURSOR = {
   'x' : 10,
   'y' : 10,
   'z' : 0,
   'char' : '#',
   'color' : libtcod.white,
   'bcolor' : libtcod.yellow
   }
Right now, it's basically just an 'editing engine'; it lets me place floors with space-bar and place objects on those floors with the enter key (and it causes an error if I try to put an object in a place where there's no Cell to 'catch' it--actually, that's a pretty critical error, so I'll go back and put some error code to catch that now).

Since the cursor ignores game-logic, it can move off the map too, so I'll add some error-catching code to stop that from happening.

EDIT: By the way, just in case I haven't made it clear recently, everyone here has been incredibly helpful and I've learned way more about Python--and programming--in the week or two since I made this thread than pretty much the one or two years I spent trying to build this engine on my own. Thanks!

EDIT-EDIT: Now I'm going through this code and adding a lot of sanity checks/error checking, so yeah. As I start building it to account for these errors and make the right decisions, I'm modifying functions accordingly.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Zamfir » Tue May 22, 2012 12:14 pm UTC

Hmm, I think your "object" class suffers from the same problem as your "properties" attribute: it's like you're reimplementing Python within Python.

[*]Your architecture idea is to have a very generic base object, with a 'properties' attribute.
[*]Then you create more specialized objects by adding properties to ' properties'.
[*]Some objects have to be similar with the same properties, so you have a global object that stores a starter set of properties, with default values
[*] And finally you have functions (like createThing) that make instances of those special objects

But if you think about it, all those things are already part of the architecture of Python itself.
[*]Your generic base object is just a normal Python object
[*] You make Python objects specilaized by giving them special properties. They are just called attributes in Python.
[*] A "class" is just a global thing that defines "default" properties for a specialized kind of object, so you can easily make for than one. Just like your BASE_CELL, except better because you define methods in the same place
[*] And classes have the __init__ method that creates instances.

See what I mean? The people who invented OOP ran into the same problems as you, and invented similar trickses to make it work. The trickses are already in the language, try to use them

Compare this:

Code: Select all
BASE_CELL = {
      'floor' : True,
      'color' : libtcod.grey,
      'inventory' : []
   }

class Object():
   pass

class Cell(Object):
   def __init__ (self, properties = {}, container = []):
      self.properties = properties
      self.container = container
   def draw(self, xyz):
      //stuff
   def clear(self, xyz):
      //stuff

def createCell(prop = {}, cont = []):
   return Cell(prop, cont)

myCell = createCell(BASE_CELL)
myCell.properties['hitPoints'] = 15


With this:
Code: Select all
class Cell():
   def __init__ (self):
      self.floor = true
      self.color =  libtcod.grey
      self.inventory = []
      self.container = [] // what does the container even do?
   def draw(self, xyz):
      //stuff
   def clear(self, xyz):
      //stuff

myCell = Cell()
mycell.hitPoints = 15

See what I am getting at? The normal structure of Python is already a near-perfect fit for your architecture, because it is based on the same ideas.

As you can see, you can even add attrbutes on the fly. You will find that this flexibility can be nice, but also dangerous. That's true for the attributes, but also for your properties. The classes help you to put a predictable structure in your code, and you are going to need every bit of structure you can find. Be flexible, but not more than necessary!

Partially, it is a naming issue. But naming is important, it's how you tell yourself what struture you are trying to make.

If you have a base class called Object, do you also expect other classes that aren't Objects? If so, what's special about classes that inherit from Object? Use that difference to give it a better name. If you cannot think of a simple name that describes how they are different from other objects, you probably should think longer about it really is supposed to do and don't.

Same for properties: what things might an object have that are not properties? How are properties different? If everything is a property, then why have a special category for it? If properties are special, different from other attributes of the class, then how are they special? Again, if you can't come up with a simple and clear description of the difference, your own mental image of the category is probably not yet sharp enough.
User avatar
Zamfir
 
Posts: 5745
Joined: Wed Aug 27, 2008 2:43 pm UTC
Location: Nederland

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Tue May 22, 2012 12:52 pm UTC

Yeah, I think I'm starting to see--I've got a lot of psychic artifacts from when I first started working on this, and part of the whole 'BASE_CELL' idea is based on the notion that I have a ton of cells populated in the game to start with, and I need to change their attributes--but part of the elegance of Python--and the Cube design--is that I can set all these attributes every time I create a cell. The same likely goes for objects.

But one danger that strikes me with this attribute method--is it possible to check for the presence of an attribute? Say my Cell doesn't have a hitpoint attribute; is there a function I can write that will check for whether or not a given attribute is present? Or do I have to check for specific attributes every time?

Basically, can I somehow do this:
Code: Select all
def hasAttribute(obj, attribute):
   return (whether or not a given attribute is there)


Or do I have to do this:
Code: Select all
def hasHitPoints(obj):
   return (whether or not the object has the hitpoint attribute)
Because that strikes me as the main advantage of the dictionary system; I can use the hasPropVal to check if a given property is stored in a given object (and whether or not the value of that property matches what I expect). And the key : value pair starts paying real dividends when I start wanting to do things like 'return every object in this inventory that has the key/value pair of 'blocked' : True. For example, here's the code I wrote to do precisely that:
Code: Select all
#returns all things with a key : value pair in a given object's inventory as a list
#returns an empty list if nothing is found
def getThings(obj, key, value = None):
   result = []
   if value == None:
      for thing in obj.properties['inventory']:
         if hasPropVal(thing, key):
            result.append(thing)
      return result
   else:
      for thing in obj.properties['inventory']:
         if hasPropVal(thing, key, value):
            result.append(thing)
      return result
(I know there's a more elegant way to do this with filters or lambda or something, but I'm still trying to work that out)

The result is a list of objects that fit a certain key-value pair (or just a list of objects that contain a certain key, ignoring its value). Could I make a function do this using just an object's attributes?

EDIT: Although part of the silliness here is that 'blocked' : True is actually unnecessary; I could set it up so 'blocked' equals anything, and just check to see if the 'blocked' key is present (its presence alone implies that an object blocks--why else would I ever set it to an object? And if I wanted it to stop, I'd just del the entry). An attribute system would probably make its value important, because I'd want all 'drawable' objects (all Things, basically) to have it. Oh, and right--'container' is a special list for the Effect object. Effect objects go inside containers and control anything to do with action--so 'poison' is an effect, and the game checks all Things' list containers every round, sees 'poison' in one of them, and applies the various attributes of poison to a Thing's properties.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Zamfir » Tue May 22, 2012 2:02 pm UTC

Code: Select all
if hasattr(myInstance, ' myProperty'):
    doStuff()

Built in and ready :)

And the key : value pair starts paying real dividends when I start wanting to do things like 'return every object in this inventory that has the key/value pair of 'blocked' : True. For example, here's the code I wrote to do precisely that:

Ah, but look carefully: you're now talking about an inventory, not properties.

As a rule of thumb (not a hard law in Python), you should use lists and dicts etc. to collect a group of similar values. So you know (at least vaguely) what sort of stuff will go in and what will come out, but their amount might vary. So an RPG-style inventory should natrually be a dict or list, depending on whether you want to retrieve the values/items by name or not.

While you use objects (and tuples) to collect a more fixed group of values that should stay together, but in return they might be very dissimilar kinds of values. A character or cell should naturally be an object, because it needs to have a given set of values to make sense, but some those values are expected to be a string, or a boolean, or a dictionary, or a Weapon instance.

You want to have both: an unpredictable amount of values, and unpredictable what those values might be. Python allows you to do this, but just because you can, doesn't mean it's a good idea. Good code is predictable and understandable, the structure of the code reflects the structure of the problem. You should use Python's flexibility to get wiggleroom in margin, not as a way to avoid structure in your code.

If your code regularly adds and removes attributes from an object, other parts of the code cannot know how the object can behave at a certain moment in time. On the other side of the coin, if you put all kinds of unpredictable stuff inside a list or dict, the rest of the code has to be afraid of what comes out of them. Unpredictable is bad.

Make structure. If all Things can be blocked or not, add an isBlocked attribute to the Thing class. Now your cade can be sure that all Things (and its children) have the attrbute. If you expect an object to ahve a number of Things, give it a List attribute called 'things', or 'inventory', or 'thingsInLeftPocket'. Only put Things (and its children) in it! Now it's predictable.
User avatar
Zamfir
 
Posts: 5745
Joined: Wed Aug 27, 2008 2:43 pm UTC
Location: Nederland

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Tue May 22, 2012 4:54 pm UTC

I've rewritten the code to try out what you're suggesting--I noticed one other advantage I lose with the dictionary-type properties, but it's not really a big one--it's harder to set all the values at once (rather than just importing a new dictionary, I have to update the attributes one-by-one; I'm sure I can write a function to automate this process, though!) or to have multiple sets of default values (rather than just one default cell, I'd like multiples--and I'd like to be able to update those multiple default cells! Basically, I'm talking about things like being able to create and place a 'stone floor', then switch over to a 'dirt floor', and have all the attributes for those two different types of cells already preset somewhere, and easily switched out).

The code ended up looking a lot cleaner, and as I wrote it, I realized a lot of odds and ends I could be doing much more efficiently--storing the x-y-z properties for Things and Cells is trivial, now (I don't need to use the key tuple, I can just throw 'em in when I make 'em), which makes things much easier to track (where is Thing B? Just check its x-y-z attributes!). I also compressed placeThing into the cell class (and changed it to 'setThing'), and I'll give the Thing class a special function that'll do the same--except put things in another thing's inventory.

One interesting consequence; I need to make sure the x-y-z attributes stay updated when a Thing moves from one cell's inventory to the next, otherwise there could be... problems. But since the draw function looks at x-y-z attributes rather than using a tuple, I should notice right away if things aren't working the way they're supposed to.

Here's the updated code!
Spoiler:
Code: Select all
import libtcodpy as libtcod
import copy
import textwrap
from config import *


class Cube(dict):
   def setGrid(self, z):
      libtcod.console_clear(mainmap)
      grid.clear()
      keys = filter(lambda key: key[2] == z, cube.keys())
      for i in keys:
         grid[i] = cube[i]
   def setCell(self, xyz, cell):
      self[(xyz)] = cell
      cell.x, cell.y, cell.z = xyz[0], xyz[1], xyz[2]
      self.setGrid(xyz[2])
   def getCell(self, xyz):
      if xyz not in self:
         return False
      return self[xyz]

class System():
   def __init__ (self):
      self.x = 1
      self.y = 1
      self.z = 0
      self.color = None
      self.bcolor = None
      self.char = None
   def draw(self):
      putChar(mainmap, self.x, self.y, self.color, self.char)
      putBack(mainmap, self.x, self.y, self.bcolor)
   def move(self, dx, dy, dz):
      def isValidLocation(x, y, z):
         return x < MAX_MAP_W and x > 0 and \
               y < MAX_MAP_H and y > 0 and \
               z > -1
      if dx == 0 and dy == 0 and dz == 0:
         return
      x, y, z = self.x, self.y, self.z
      x, y, z = x+dx, y+dy, z+dz
      if not isValidLocation(x, y, z):
         return
      self.clear()
      self.x, self.y, self.z = x, y, z
   def clear(self):
      putChar(mainmap, self.x, self.y, libtcod.black, " ")
      putBack(mainmap, self.x, self.y, libtcod.black)

class Cell():
   def __init__ (self, xyz, container = []):
      self.container = container
      self.x = xyz[0]
      self.y = xyz[1]
      self.z = xyz[2]
      self.color = libtcod.grey
      self.bcolor = libtcod.grey
      self.char = None
      self.floor = True
      self.inventory = []
   def draw(self):
      self.clear() #<---unnecessary? Remember this will clear Things too
      putBack(mainmap, self.x, self.y, self.bcolor)
      putChar(mainmap, self.x, self.y, self.color, self.char)
      for thing in self.inventory:
         thing.draw()
   def clear(self):
      putBack(mainmap, self.x, self.y, libtcod.black)
      putChar(mainmap, self.x, self.y, libtcod.black, " ")
   def setThing(self, thing):
      thing.x, thing.y, thing.z = self.x, self.y, self.z
      self.inventory.append(thing)
   #WARNING! Removing a thing without setting it somewhere else leaves it without *ANY* pointer!
   def removeThing(self, thing):
      try:
         self.inventory.remove(thing)
      except ValueError:
         return

class Thing():
   def __init__ (self, xyz, container = []):
      self.container = container
      self.x = xyz[0]
      self.y = xyz[1]
      self.z = xyz[2]
      self.color = libtcod.white
      self.char = '@'
      self.blocked = True
   def draw(self):
      putChar(mainmap, self.x, self.y, self.color, self.char)

#temporary, may be thoroughly modified
class Effect():
   def __init__ (self, name, action = []):
      self.name = name
      self.action = action

def start():
   global mainmap, cube, grid, cursor, focus
   libtcod.console_set_custom_font('arial10x10.png', libtcod.FONT_TYPE_GREYSCALE | libtcod.FONT_LAYOUT_TCOD)
   libtcod.console_init_root(CON_W, CON_H, 'ROGUE-CLONE ENGINE', False)
   mainmap = libtcod.console_new(MAX_MAP_W, MAX_MAP_H)
   cube = Cube()
   grid = {}
   cursor = System()
   cursor.x, cursor.y, cursor.z, cursor.char, cursor.color, cursor.bcolor = 5, 5, 0, None, None, libtcod.yellow
   focus = cursor
   cube.setGrid(focus.z)
   renderAll()

#may need to be expanded for graphic effects
def putChar(target, x, y, color, char):
   libtcod.console_set_foreground_color(target, color)
   libtcod.console_put_char(target, x, y, char, libtcod.BKGND_NONE)

#may need to be expanded for graphic effects
def putBack(target, x, y, color):
   libtcod.console_set_back(target, x, y, color)

def renderAll():
   for cell in grid:
      grid[cell].draw()
   focus.draw()
   libtcod.console_blit(mainmap, 0, 0, CON_W, CON_H, 0, 0, 0)

#might need expansion
def createThing(xyz):
   return Thing(xyz)

#might need expansion
def createCell(xyz):
   return Cell(xyz)

def getKey(key):
   if key.vk == libtcod.KEY_CHAR:
      return chr(key.c)
   else:
      return key.vk

#handle user input
def handle_keys():
   #just to keep things shorter and more concise
   xyz, z = (focus.x, focus.y, focus.z), focus.z
   cell = cube.getCell(xyz) #<--current cell we're in (False if we aren't in one)
   key = getKey(libtcod.console_wait_for_keypress(True))

   if libtcod.console_is_key_pressed(libtcod.KEY_ESCAPE):
      return 'True'
   if libtcod.console_is_key_pressed(libtcod.KEY_SPACE):
      cell = createCell(xyz)
      cube.setCell(xyz, cell)

   if libtcod.console_is_key_pressed(libtcod.KEY_ENTER):
      if cell:
         cell.setThing(createThing(xyz))

   #to test z-level rendering
   if libtcod.console_is_key_pressed(libtcod.KEY_TAB):
      focus.move(0, 0, 1)
      cube.setGrid(focus.z)

   #to test z-level rendering
   if key == "z":
      focus.move(0, 0, -1)
      cube.setGrid(focus.z)

   if key == "t":
      pass

   if key in MOVE_KEYS:
      dx, dy, dz = MOVE_KEYS[key]
      focus.move(dx, dy, dz)



start()
#main game loop
while not libtcod.console_is_window_closed():

   libtcod.console_flush()
   exit = handle_keys()
   renderAll()
   print str(focus.x), " ", str(focus.y), " ", str(focus.z)
   if exit:
      break
I kept the other format saved, just in case I end up switching back to the properties[dict] system, but this does seem to work--and read!--much more cleanly.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby EvanED » Tue May 22, 2012 5:17 pm UTC

Again not reading too much because there are a lot of long posts in this thread, one thing I note in several places: def foo(x = []):. This is almost guaranteed to be wrong.

Here's what winds up happening:
Code: Select all
>>> class C(object):
...     def __init__(self, container = []):
...         self.container = container
...
>>> c1 = C()
>>> c2 = C()
>>> c1.container
[]
>>> c2.container
[]
>>> c1.container.append(12)
>>> c1.container
[12]
>>> c2.container
[12]                                          # !!!!!

This is because it doesn't create a new [] for each call to the constructor, so both objects have the same one:
Code: Select all
>>> c1.container is c2.container
True


I'd go so far as to say that mutable types as default parameters are essentially always a bad idea for this reason. (Even if it's correct because the objects are never mutated and no reference escapes the function, you still have to think about it to make sure that's true.)

Use the following idiom instead:
Code: Select all
def foo(container = None):
    if container is None:
        container = []
    ...

or, if None is a "valid" parameter value:
Code: Select all
_sentinel = object()
def foo(container = _sentinel):
    if container is _sentinel:
        container = []
    ...
EvanED
 
Posts: 3767
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Tue May 22, 2012 5:23 pm UTC

Oops; had no idea that it could do that. Thanks for the catch! Fixed in the constructors.

EDIT: This new approach is really working well. Thing movement is up and fine; Things now inherit the properties of Cells (since there's some overlap of their functionality), Cells inherit the 'blocked' attribute from Things from inside them (and have it removed when the Thing is taken out), I can change the focus (who's moving) with one line of code... now I just have to figure out the next big thing. I'm guessing setting it up so I can build, save, and load maps--so I can start working on some of the nitty-gritty architecture of how Things interact with other Things (and how it affects Cells).

EDIT-EDIT: And I just learned about the setattr(object, string, value), which lets me set values for attributes (even if they're not there). In other words, it's easy-peasy to create a custom type dictionary then set the attributes of an object to the contents of that dictionary.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Ben-oni » Tue May 22, 2012 5:58 pm UTC

The Great Hippo wrote:
Ben-oni wrote:As for using an object as a shortcut to it's properties, you had good reasons for using a properties dictionary within the object. But a bit of shorthand, as recommended, would be handy:
Code: Select all
class Object:
   def __init__(self, props = {}):
      self.properties = args
      self.container = []
   
   def __setitem__(self, key, value):
      self.property[key] = value
   
   def __getitem__(self, key):
      return self.property[key]
Wait--so can you give me an example of how I'd retrieve a property from this Object class? Say I wanted the list in the 'inventory' property for an object called 'Cell'. Would I type 'Cell['inventory']' with this code? I assume I could assign it the same way?

Part of what's throwing me off is the 'property' versus 'properties' tag. Is property a built-in type? I don't get where or how the class here 'knows' what property means (or does it even care? Is 'property' is just a descriptive name, and you're just telling the code 'hey, store this type of data-structure here'?). I think I need to study classes a bit more--because constructors like these keep throwing me off course.

My mistake. "property" should be "properties".

@The Great Hippo: As for using Python's object system rather than your own... it's probably a bad idea. Python happens to have 90% of the features you want from your object system. The other 10% can probably be approximated down the road. Maybe. But don't count on it.

@Zamfir: The OP wants a very flexible object system. The very first post explains that objects have properties and effects (though he wasn't naming them as such, yet), and he wanted to be able to serialize them. In particular, an object "inherits" properties from effects. This is something that Python's object system would have a very hard time with. While he could manually copy over properties, he'll eventually want to just check for the presence of a key in objects in the container, and use them as surrogates.


The takeaway here: don't shoehorn a programs natural structure into language specific constructs like an object system. It's a bad practice, and makes you look like a two penny whore.
Ben-oni
 
Posts: 268
Joined: Mon Sep 26, 2011 4:56 am UTC

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby EvanED » Tue May 22, 2012 6:04 pm UTC

Also note that Ben-oni's example makes the same mistake re. mutable default parameters. :-)
EvanED
 
Posts: 3767
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Tue May 22, 2012 6:16 pm UTC

Ben-oni wrote:@Zamfir: The OP wants a very flexible object system. The very first post explains that objects have properties and effects (though he wasn't naming them as such, yet), and he wanted to be able to serialize them. In particular, an object "inherits" properties from effects. This is something that Python's object system would have a very hard time with. While he could manually copy over properties, he'll eventually want to just check for the presence of a key in objects in the container, and use them as surrogates.


The takeaway here: don't shoehorn a programs natural structure into language specific constructs like an object system. It's a bad practice, and makes you look like a two penny whore.
Is there a way I could substitute an object-specific dictionary for the properties attribute? Maybe make all my objects inherit the dict class? So instead of Cell.x--or Cell.properties['x']--it would be Cell['x'], and I'd define these properties in the __init__ script.
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Ben-oni » Tue May 22, 2012 6:18 pm UTC

EvanED wrote:Also note that Ben-oni's example makes the same mistake re. mutable default parameters. :-)

That's specifically taken from the OP's existing code, just in case he had good reason for it. I have seen scenarios that rely on the default value being the same across calls. I was actually tempted to use **args for the dictionary, just to see how he'd respond to that.

EDIT:

The Great Hippo wrote:Is there a way I could substitute an object-specific dictionary for the properties attribute? Maybe make all my objects inherit the dict class? So instead of Cell.x--or Cell.properties['x']--it would be Cell['x'], and I'd define these properties in the __init__ script.

Yes, already demonstrated, but without inheritance. __setitem__ and __getitem__ do the job.
Ben-oni
 
Posts: 268
Joined: Mon Sep 26, 2011 4:56 am UTC

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Tue May 22, 2012 6:54 pm UTC

Ben-oni wrote:Yes, already demonstrated, but without inheritance. __setitem__ and __getitem__ do the job.
I wasn't able to get the code you gave me to work, even with the property/properties fix--I klutzed around with it and got the following to work (I think?):
Code: Select all
class Object:
   def __init__(self, props = None):
      if props is None:
         self.properties = {}
      else:
         self.properties = props
      self.container = []
      self.properties['z'] = "HEY!" 

   def __setitem__(self, key, value):
      self.properties[key] = value
   
   def __getitem__(self, key):
      return self.properties[key]

junk = Object()
junk2 = Object()
junk['x'] = "hello"
junk['y'] = "bye"
junk['z'] = "crap"
print str(junk['x'])
print str(junk['y'])
print str(junk2['z'])
print str(junk.properties.keys())
Which produces the expected output for the values given. Just to make sure--before I implement this--am I making any critical errors here? Was there a reason why args should be included that I'm not getting?
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby Ben-oni » Tue May 22, 2012 7:03 pm UTC

The Great Hippo wrote:
Ben-oni wrote:Yes, already demonstrated, but without inheritance. __setitem__ and __getitem__ do the job.
I wasn't able to get the code you gave me to work, even with the property/properties fix--I klutzed around with it and got the following to work (I think?):
Code: Select all
class Object:
   def __init__(self, props = None):
      if props is None:
         self.properties = {}
      else:
         self.properties = props
      self.container = []
      self.properties['z'] = "HEY!" 

   def __setitem__(self, key, value):
      self.properties[key] = value
   
   def __getitem__(self, key):
      return self.properties[key]

junk = Object()
junk2 = Object()
junk['x'] = "hello"
junk['y'] = "bye"
junk['z'] = "crap"
print str(junk['x'])
print str(junk['y'])
print str(junk2['z'])
print str(junk.properties.keys())
Which produces the expected output for the values given. Just to make sure--before I implement this--am I making any critical errors here? Was there a reason why args should be included that I'm not getting?

Oh, man. Yeah, I'd been messing with a couple ways of doing it, and forgot to clean that out. I should have just skipped __init__ altogether. You're good now, mostly. About using None or _sentinel, EvanED is also wrong. If the value exists, it will be passed by someone sufficiently crazy. Try this:

Code: Select all
   def __init__(self, *props):
      self.properties = {} if len(props)==0 else props[0]
      self.container = []
      self.properties['z'] = "HEY!" 
Ben-oni
 
Posts: 268
Joined: Mon Sep 26, 2011 4:56 am UTC

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby The Great Hippo » Tue May 22, 2012 7:10 pm UTC

That works too--the asterisk keeps throwing me, though. I know what it means in the case of *args (I think), but every tutorial I've found on it only talks about that case; in this case, is it signifying that the function in question doesn't need that value? Or that value could be multiple arguments? Or... (I'm assuming args is a built-in type, and does a specific thing--is args just the protocol, and it's the asterisk that actually performs its function?)

(On that note, what happens if props is an actual dictionary? Let me try that...

Yep, imports the whole dictionary!)

EDIT: Oh, hey--can I actually access the __setitem__ constructor while I'm still performing the __init__ constructor? If so, I don't even need to type 'self.properties['x'] = 1' in the initialization; I can just type self['x'] = 1. This seems to work in my test version; is there any reason to suspect doing this would be a problem later on? I can't imagine why.

EDIT-EDIT: Also, cool and unexpected consequence--instead of having to do a loop to set my attributes one by one when I want to change all of an object's properties on one go, I just have to do setattr once, target the properties attribute, and replace that dictionary with a new one. I also finally 'get' (I think!) what the constructors are actually doing, and what __setitem__, __getitem__, and __setattr__ actually mean. I might check __main__ later to get a better grasp over how the various datatypes use these variables to accomplish their tasks.

EDIT-EDIT-EDIT: Last one, I promise. Just wanted to mention--made the changes, ran the engine, everything runs smooth. Here's the updated code:
Spoiler:
Code: Select all
import libtcodpy as libtcod
import copy
import textwrap
from config import *


class Cube(dict):
   def setGrid(self, z):
      libtcod.console_clear(mainmap)
      grid.clear()
      keys = filter(lambda key: key[2] == z, cube.keys())
      for i in keys:
         grid[i] = cube[i]
   def setCell(self, xyz, cell):
      self[(xyz)] = cell
      cell['x'], cell['y'], cell['z'] = xyz[0], xyz[1], xyz[2]
      self.setGrid(xyz[2])
   def getCell(self, xyz):
      if xyz not in self:
         return False
      return self[xyz]

class BaseObject():
   def __init__(self, *props):
      self.properties = {} if len(props)==0 else props[0]

   def __setitem__(self, key, value):
      self.properties[key] = value
   
   def __getitem__(self, key):
      return self.properties[key]


class System(BaseObject):
   def __init__ (self, *props):
      self.properties = {} if len(props)==0 else props[0]      
      self['x'] = 1
      self['y'] = 1
      self['z'] = 0
      self['color'] = None
      self['bcolor'] = None
      self['char'] = None
   def draw(self):
      putChar(mainmap, self['x'], self['y'], self['z'], self['char'])
      putBack(mainmap, self['x'], self['y'], self['bcolor'])
   def move(self, dx, dy, dz):
      def isValidLocation(x, y, z):
         return x < MAX_MAP_W and x > 0 and \
               y < MAX_MAP_H and y > 0 and \
               z > -1
      if dx == 0 and dy == 0 and dz == 0:
         return
      x, y, z = self['x'], self['y'], self['z']
      x, y, z = x+dx, y+dy, z+dz
      if not isValidLocation(x, y, z):
         return
      self.clear()
      self['x'], self['y'], self['z'] = x, y, z
   def clear(self):
      putChar(mainmap, self['x'], self['y'], libtcod.black, " ")
      putBack(mainmap, self['x'], self['y'], libtcod.black)

class Cell(BaseObject):
   def __init__ (self, xyz, *props):
      self.properties = {} if len(props)==0 else props[0]
      self.container = []
      self['x'] = xyz[0]
      self['y'] = xyz[1]
      self['z'] = xyz[2]
      self['color'] = libtcod.grey
      self['bcolor'] = libtcod.grey
      self['char'] = None
      self['floor'] = True
      self['inventory'] = []
      self['blocked'] = False
   def draw(self):
      self.clear() #<---unnecessary? Remember this will clear Things too
      putBack(mainmap, self['x'], self['y'], self['bcolor'])
      putChar(mainmap, self['x'], self['y'], self['color'], self['char'])
      for thing in self['inventory']:
         thing.draw()
   def clear(self):
      putBack(mainmap, self['x'], self['y'], libtcod.black)
      putChar(mainmap, self['x'], self['y'], libtcod.black, " ")
   def getThing(self, thing):
      try:
         index = self['inventory'].index(thing)
         return self['inventory'][index]
      except ValueError: #<--this might need work, since it RETURNS EMPTY if Thing is not found
         return
   def setThing(self, thing):
      thing['x'], thing['y'], thing['z'] = self['x'], self['y'], self['z']
      self['blocked'] = thing['blocked'] #<--items that block put in non-blocking items turn them into blocking items
      self['inventory'].append(thing)
   #WARNING! Removing a thing without setting it somewhere else leaves it without *ANY* pointer!
   def removeThing(self, thing):
      try:
         self['inventory'].remove(thing)
      except ValueError:
         return

class Thing(Cell):
   def __init__ (self, xyz, *props):
      self.properties = {} if len(props)==0 else props[0]
      self.container = []
      self['x'] = xyz[0]
      self['y'] = xyz[1]
      self['z'] = xyz[2]
      self['color'] = libtcod.white
      self['bcolor'] = libtcod.white
      self['char'] = None
      self['blocked'] = True
   def draw(self):
      if self['char'] != None:
         putChar(mainmap, self['x'], self['y'], self['color'], self['char'])
      if self['bcolor'] != None:
         putBack(mainmap, self['x'], self['y'], self['bcolor'])
   def move(self, dx, dy, dz):         
      if dx == 0 and dy == 0 and dz == 0:
         return
      x, y, z = self['x'], self['y'], self['z']
      x, y, z = x+dx, y+dy, z+dz
      cell = cube.getCell((x, y, z))
      oldcell = cube.getCell((self['x'], self['y'], self['z']))
      if cell:
         if cell['blocked'] == True:
            return
         self.clear()
         oldcell['blocked'] = False
         self['x'], self['y'], self['z'] = x, y, z
         cell.removeThing(self)
         cell.setThing(self)
         cell.blocked = self['blocked']


#temporary, may be thoroughly modified
class Effect(BaseObject):
   pass

def start():
   global mainmap, cube, grid, cursor, focus
   libtcod.console_set_custom_font('arial10x10.png', libtcod.FONT_TYPE_GREYSCALE | libtcod.FONT_LAYOUT_TCOD)
   libtcod.console_init_root(CON_W, CON_H, 'ROGUE-CLONE ENGINE', False)
   mainmap = libtcod.console_new(MAX_MAP_W, MAX_MAP_H)
   cube = Cube()
   grid = {}
   cursor = System()
   setType(cursor, CURSOR)
   focus = cursor
   cube.setGrid(focus['z'])
   renderAll()

#may need to be expanded for graphic effects
def putChar(target, x, y, color, char):
   libtcod.console_set_foreground_color(target, color)
   libtcod.console_put_char(target, x, y, char, libtcod.BKGND_NONE)

#may need to be expanded for graphic effects
def putBack(target, x, y, color):
   libtcod.console_set_back(target, x, y, color)

def renderAll():
   for cell in grid:
      grid[cell].draw()
   focus.draw()
   libtcod.console_blit(mainmap, 0, 0, CON_W, CON_H, 0, 0, 0)

#might need expansion
def createThing(xyz):
   return Thing(xyz)

#might need expansion
def createCell(xyz):
   return Cell(xyz)

def getKey(key):
   if key.vk == libtcod.KEY_CHAR:
      return chr(key.c)
   else:
      return key.vk

def setType(obj, my_dict):
   setattr(obj, 'properties', my_dict)

#handle user input
def handle_keys():
   global focus, player
   #just to keep things shorter and more concise
   xyz, z = (focus['x'], focus['y'], focus['z']), focus['z']
   cell = cube.getCell(xyz) #<--current cell we're in (False if we aren't in one)
   key = getKey(libtcod.console_wait_for_keypress(True))

   if libtcod.console_is_key_pressed(libtcod.KEY_ESCAPE):
      return 'True'
   if libtcod.console_is_key_pressed(libtcod.KEY_SPACE):
      cell = createCell(xyz)
      cube.setCell(xyz, cell)

   if libtcod.console_is_key_pressed(libtcod.KEY_ENTER):
      if cell:
         cell.setThing(createThing(xyz))

   #to test z-level rendering
   if libtcod.console_is_key_pressed(libtcod.KEY_TAB):
      focus.move(0, 0, 1)
      cube.setGrid(focus['z'])

   #to test z-level rendering
   if key == "z":
      focus.move(0, 0, -1)
      cube.setGrid(focus['z'])

   if key == "e":
      focus = cursor

   if key == "t": #Test the map!
      if cell:
         player = createThing(xyz)
         setType(player, PLAYER)
         cell.setThing(player)
         focus = player

   if key in MOVE_KEYS:
      dx, dy, dz = MOVE_KEYS[key]
      focus.move(dx, dy, dz)



start()

#main game loop
while not libtcod.console_is_window_closed():

   libtcod.console_flush()
   exit = handle_keys()
   renderAll()

   if exit:
      break
(Probably don't need the initialization code for BaseObject--or I could just make it the initialization code for Effect, since everything else overwrites BaseObject's initialization constructor--we'll see! A lot of the sets I'm doing in the initialization structors are probably just temporary; I'm going to build a sort of 'type' system to create default objects with the setType function so I can have a lot more flexibility when I start building maps.)
User avatar
The Great Hippo
 
Posts: 5489
Joined: Fri Dec 14, 2007 4:43 am UTC
Location: behind you

Re: (Newbie Warning) Coding Rogue-Clone in Python

Postby EvanED » Tue May 22, 2012 7:52 pm UTC

Ben-oni wrote:About using None or _sentinel, EvanED is also wrong. If the value exists, it will be passed by someone sufficiently crazy.

I'm giving the standard advice. If it's not meaningful to pass None, then you can use that as your sentinel. You should document it of course, but you always need to document default parameters with hidden values like this anyway, because they'll never be visible in the function signature. And then you just say "if arg is omitted or None, then ...".

In the case of _sentinel, the fact that someone is using an _ identifier means that either they've looked at your code reasonable carefully or I don't care what happens to them. Pretty much by definition they're trying to break your code.

Or do the _sentinel trick but name it something like DEFAULT instead (or an even more meaningful name). I've seen that too.

Any of these solutions is far more idiomatic and, IMO, easier to understand than what you suggest.

But here are some other concrete advantages my suggestion has:
  • My version will at least somewhat work with code inspection tools that look at your function signatures to provide nice messages -- which your *props breaks. (With my version, they'll show you what that parameter is and that it's there, just not its default value. With your version, they'll just show "some arguments go here. maybe.")
  • My version also allows you to pass the argument as a keyword argument, which yours won't.
  • My version will also catch calling the function with too many arguments, which yours won't.
  • My version will give a better diagnostic in the case of calling it with too few arguments than yours will.
(The last two, of course, are easily fixed, but that takes more code, and it's code that my version doesn't need.)

The Great Hippo wrote:That works too--the asterisk keeps throwing me, though. I know what it means in the case of *args (I think), but every tutorial I've found on it only talks about that case; in this case, is it signifying that the function in question doesn't need that value? Or that value could be multiple arguments? Or... (I'm assuming args is a built-in type, and does a specific thing--is args just the protocol, and it's the asterisk that actually performs its function?)

args is just a name, no different than any other name. It's the * that makes it work.
EvanED
 
Posts: 3767
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI

PreviousNext

Return to Coding

Who is online

Users browsing this forum: MobTeeseboose, Slageammalymn and 12 guests