patternpythonMinor
Text-based RPG using OOP
Viewed 0 times
textusingbasedrpgoop
Problem
I started this project after reading Chapter 3 of Python 3 Object-Oriented Programming by Dusty Phillips. At first I just wanted to create RPG-based objects to practice inheritance, but now I am more interested with seeing how far I can take this RPG project. Just for reference, I'll say that I am a beginner programming student, and while I am decent at solving problems with code, I am completely new to building programs.
```
######## Python Text Based RPG adventure #########
import time
import random
class Character(object):
def __init__(self, name='', hp=0, attack=0, **kwargs):
super().__init__(**kwargs)
self.name = name
self.hp = hp
self.attack = attack
def Attack(self, other):
print(self.name + " is attacking " + other.name + "!!!!")
other.hp -= self.attack
print('you dealt ' + str(self.attack) + ' damage!!')
class Enemy(Character):
def __init__(self, **kwargs):
super().__init__(**kwargs)
class SadSad(Enemy):
'''This is me considering if I should make
seperate classes for each enemy instance, or
maybe make a function that generates random Enemies'''
def __init__(self, **kwargs):
super().__init__(**kwargs)
self.name = "SadSad"
self.hp = 5
self.attack = 1
class Player(Character):
def __init__(self, weapons=None, armor=None, inventory=[], **kwargs):
super().__init__(**kwargs)
self.weapons = weapons
self.armor = armor
self.inventory = inventory
def inspect(self):
print(self.name + ' has ' + \
str(self.hp) + ' HP and ' + \
str(self.attack) + ' Attack.')
@staticmethod
def inspect_other(other):
print(other.name + ' has ' + \
str(other.hp) + ' HP and ' + \
str(other.attack) + ' Attack.')
def collect(self, item):
'''puts any existing item into the Player's inventory'''
if item.state == None:
```
######## Python Text Based RPG adventure #########
import time
import random
class Character(object):
def __init__(self, name='', hp=0, attack=0, **kwargs):
super().__init__(**kwargs)
self.name = name
self.hp = hp
self.attack = attack
def Attack(self, other):
print(self.name + " is attacking " + other.name + "!!!!")
other.hp -= self.attack
print('you dealt ' + str(self.attack) + ' damage!!')
class Enemy(Character):
def __init__(self, **kwargs):
super().__init__(**kwargs)
class SadSad(Enemy):
'''This is me considering if I should make
seperate classes for each enemy instance, or
maybe make a function that generates random Enemies'''
def __init__(self, **kwargs):
super().__init__(**kwargs)
self.name = "SadSad"
self.hp = 5
self.attack = 1
class Player(Character):
def __init__(self, weapons=None, armor=None, inventory=[], **kwargs):
super().__init__(**kwargs)
self.weapons = weapons
self.armor = armor
self.inventory = inventory
def inspect(self):
print(self.name + ' has ' + \
str(self.hp) + ' HP and ' + \
str(self.attack) + ' Attack.')
@staticmethod
def inspect_other(other):
print(other.name + ' has ' + \
str(other.hp) + ' HP and ' + \
str(other.attack) + ' Attack.')
def collect(self, item):
'''puts any existing item into the Player's inventory'''
if item.state == None:
Solution
You have bugs, bugs everywhere!
To show this bug, I'll make a function, that takes some data, mutates it a little and returns it. If we pass nothing to it, it should return just the mutation.
So to keep it simple we append
But I'll show you the bug.
Yes, you can test it yourself. That is not what you want.
And you should be able to see every time we call
This is as all function calls share the same instance of the list.
Which is why
To fix this you should change all default parameters to
And overwrite it with a new instance of a list.
And so I'd take a leap out of JavaScripts book and use
Allowing for:
Or if you don't like that a more verbose, but probably a better function:
But ultimately it's your choice. I'm going to use
Your classes are good. But I'd change them. A lot.
-
The class
But if you really need this class I'd say use
This is as you then don't need to write all the boilerplate to make a class, and you can create all the classes on one or two lines using a comprehension.
You may prefer the
The reason I'd still make the minion a type, is as you can then just make a ton of them.
But I'd not make them a class as you aren't going to and don't add anything new.
I'd also not do
-
I'd move
which already has all the attributes in
I'd also remove the
-
I'd re-organise your code so that you group classes. You broke the inventory management group by putting
And this threw me off when I was reading your code.
-
I'd finally add an
All the additional code that you added to the
And so I'd write a new class to handle this, so that if you need to add it to a special NPC you can add it with ease.
To make it somewhat easy to add I'd add it to a class via inheritance. This has the down-side of you having to manually initialize it.
But it allows you to add two things and for the code to be fine.
If I didn't like this idea then I would make it a stand alone class and manually instantiate it and call it from the child class.
This allows a separation of the two objects, which doesn't make too much sense, if the whole point of the class it to allow you to easily add a feature to an existing class.
Either way, to change to and from each other is easy.
I made some changes to the classes as I didn't like how you implemented some of the code.
If we look at
Instead I'd make a function that tells us which slot the object belongs in, then a function that actually equips the item, and the original function who's purpose is to interface these two functions.
I also removed the print functions from this function, as I don't like the idea of printing and mutating data. As it violates SRP.
I'd also like to mention that
And I used the same approach as I did for equipping the item.
This resulted in me getting the following
```
class Equipment:
def __init__(self, equipment=None, inventory=None):
self.equipment = equipment or {}
self.inventory = inventory or []
def collect(self, item):
i
inventory=[] looks innocent, nothing possibly can go wrong here! But it's actually very different to how I think you think it works.To show this bug, I'll make a function, that takes some data, mutates it a little and returns it. If we pass nothing to it, it should return just the mutation.
So to keep it simple we append
'test' to it. So test() -> ['test'] and test(['this', 'is', 'a']) -> ['this', 'is', 'a', 'test'].But I'll show you the bug.
>>> def test(data=[]):
data.append('test')
return data
>>> test()
['test']
>>> test(['this', 'is', 'a'])
['this', 'is', 'a', 'test']
>>> test() == test()
True
>>> test() is test()
True
>>> test()
['test', 'test', 'test', 'test', 'test', 'test']Yes, you can test it yourself. That is not what you want.
And you should be able to see every time we call
test() it adds one 'test' to the list.This is as all function calls share the same instance of the list.
Which is why
test() is test() is True.To fix this you should change all default parameters to
None.And overwrite it with a new instance of a list.
And so I'd take a leap out of JavaScripts book and use
or to the overwrite the list.Allowing for:
>>> def test(data=None):
data = data or []
data.append('test')
return data
>>> test()
['test']
>>> test()
['test']
>>> test(['this', 'is', 'a'])
['this', 'is', 'a', 'test']Or if you don't like that a more verbose, but probably a better function:
>>> def verb_test(data=None):
if data is None:
data = []
data.append('test')
return data
>>> l = []
>>> test(l)
['test']
>>> l
[]
>>> verb_test(l)
['test']
>>> l
['test']But ultimately it's your choice. I'm going to use
or as it makes smaller code.Your classes are good. But I'd change them. A lot.
-
The class
SadSad is bad as it's just going to end up as an instance of Enemy.But if you really need this class I'd say use
type, SadSad = type('SadSad', (object,), {}).This is as you then don't need to write all the boilerplate to make a class, and you can create all the classes on one or two lines using a comprehension.
You may prefer the
class way however.The reason I'd still make the minion a type, is as you can then just make a ton of them.
But I'd not make them a class as you aren't going to and don't add anything new.
I'd also not do
goblin = Enemy(name="goblin", hp=50, attack=5). As then you can't make other goblins, and there is no hierarchy.-
I'd move
inspect into Character, and remove inspect_other. This is as inspect_other is going to be used to inspect a Character or a subclass of it,which already has all the attributes in
inspect.I'd also remove the
inspect_other as it's a duplicate of inspect. And should have no use after moving inspect to Character.-
I'd re-organise your code so that you group classes. You broke the inventory management group by putting
Room between Player and Item.And this threw me off when I was reading your code.
-
I'd finally add an
Equipment class that handles adding and removing items from your equipment and inventory.All the additional code that you added to the
Player class modifies these properties.And so I'd write a new class to handle this, so that if you need to add it to a special NPC you can add it with ease.
To make it somewhat easy to add I'd add it to a class via inheritance. This has the down-side of you having to manually initialize it.
But it allows you to add two things and for the code to be fine.
If I didn't like this idea then I would make it a stand alone class and manually instantiate it and call it from the child class.
This allows a separation of the two objects, which doesn't make too much sense, if the whole point of the class it to allow you to easily add a feature to an existing class.
Either way, to change to and from each other is easy.
I made some changes to the classes as I didn't like how you implemented some of the code.
If we look at
equip_gear you should be able to see that there is code duplication, and the only change is that you changed from self.weapon to self.armor.Instead I'd make a function that tells us which slot the object belongs in, then a function that actually equips the item, and the original function who's purpose is to interface these two functions.
I also removed the print functions from this function, as I don't like the idea of printing and mutating data. As it violates SRP.
I'd also like to mention that
unequip wouldn't allow us to re-equip the gear, as it never changed the state of the item.And I used the same approach as I did for equipping the item.
This resulted in me getting the following
Equipment class:```
class Equipment:
def __init__(self, equipment=None, inventory=None):
self.equipment = equipment or {}
self.inventory = inventory or []
def collect(self, item):
i
Code Snippets
>>> def test(data=[]):
data.append('test')
return data
>>> test()
['test']
>>> test(['this', 'is', 'a'])
['this', 'is', 'a', 'test']
>>> test() == test()
True
>>> test() is test()
True
>>> test()
['test', 'test', 'test', 'test', 'test', 'test']>>> def test(data=None):
data = data or []
data.append('test')
return data
>>> test()
['test']
>>> test()
['test']
>>> test(['this', 'is', 'a'])
['this', 'is', 'a', 'test']>>> def verb_test(data=None):
if data is None:
data = []
data.append('test')
return data
>>> l = []
>>> test(l)
['test']
>>> l
[]
>>> verb_test(l)
['test']
>>> l
['test']class Equipment:
def __init__(self, equipment=None, inventory=None):
self.equipment = equipment or {}
self.inventory = inventory or []
def collect(self, item):
if item.state == None:
self.inventory.append(item)
return True
return False
@staticmethod
def _get_slot(item):
types = [(Weapon, 'weapon'), (Armor, 'armor')]
for class_, slot in types:
if isinstance(item, class_):
return slot
return None
def equip(self, item):
slot = self._get_slot(item)
if slot is None:
return False
return self._equip(item, slot)
def _equip(self, item, slot):
if self.equipment[slot] is None and item in self.inventory:
self.equipment[slot] = item
item.state = True
self.inventory.remove(item)
return True
return False
def unequip(self, item):
slot = self._get_slot(item)
if slot is None:
return False
return self._unequip(item, slot)
def _unequip(self, item, slot):
if item == self.equipment[slot]:
item = self.equipment[slot]
self.inventory.append(item)
self.equipment[slot] = None
item.state = None
return True
return Falseclass Player(Character, Equipment):
def __init__(self, equipment=None, inventory=None, **kwargs):
super().__init__(**kwargs)
Equipment.__init__(self, equipment, inventory)
@property
def attack(self):
return self._attack + self.equipment['weapon'].attack
def collect(self, item):
if not super().collect(item):
print('You cannot collect that.')
def equip(self, item):
if not super().equip(item):
print('That cannot be equipped.')Context
StackExchange Code Review Q#138085, answer score: 2
Revisions (0)
No revisions yet.