HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Shortcuts and imports for large RPG basic code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
importsshortcutscodelargeforrpgandbasic

Problem

I decided to work on putting together an Arena-style (very basic) text-based RPG as a way to help myself learn Python. Unfortunately, after about 1,000 lines of pieced-together code, I realize that I've been doing myself a disservice by reenforcing poor practices, and by going line-by-line, my inefficiency is multiplying exponentially.

I don't honestly know where to start. I'm sure there are libraries, shortcuts, and greatly more efficient methods than what I'm using. I'm okay with starting over (there are errors not worth finding, anyway), but I would hugely appreciate anyone knowledgeable looking over my code, giving me a few of the bigger pointers for how to clean up my current methods, and where to look for new ones. A quick skim, I'm sure, would suffice. I also want to keep it mostly text and basic, so I'm not looking for something like PyGame, just straightforward stuff. (For extra, I plan on utilizing my final code to create a "leaderboard" of randomly-generated characters that fight each other, so pointers in that direction help). I had to cut out many of the very long lists for post length, but it gives you an idea.

```
import random
import math

class character:

def __init__(self):

# There are hundreds of these... cut down for post length
self.STR = 75
self.DEX = 75
self.CON = 75
self.AGI = 75
self.INT = 75
self.WIS = 75
self.statPoints = 75
self.AP = 0
self.XP = 0
self.LVL = 1
self.name = ''
self.xpToLVL = 500
self.fightsWon = 0
self.XPSinceLVL = 0
self.dmgmitPercent = 0
self.mindmg = 0
self.maxdmg = 0
self.mindmgmit = 0
self.maxdmgmit = 0
self.minrobeSP = 0
self.maxrobeSP = 0
self.WarBuff = 0
self.poisonCounter = 0
self.dmg = 0
self.armorType = 0
self.shieldEquipped = 0
self.visitedPTrainer = 0
self.visitedCT

Solution

The primary problem I see is that your functions try to do everything from start to finish. For example fightRound knows all about the different kinds of weapons and armor and chances of critical hits and what effect these all have on the damage. It would benefit greatly being separated into multiple items that handle individual aspects of this. Consider a conductor of an orchestra: he doesn't tell violinist to pull the bow or the trumpet player when to breathe; the conductor just decides when the notes must be played.

You can make fightRound more like a conductor through something called refactoring. It's a process of finding similar code, turning it into reusable blocks, and using that new block without changing the meaning of your code. By giving that reusable block a name, it can become the master of its primary purpose (playing a trumpet note), and let the caller focus on conducting.

In your code you can find many good candidates for refactoring by looking for if/elif trees. Since I'm not seeing all of your code, I may guess the wrong pattern to simplify. But on the whole this approach should help reduce the amount of code, and increase the focus within each block. But watch out for any cases where my advice is wrong because I didn't have the full picture, and see if you can figure out how to tweak it into something that helps.

Refactoring into functions

Let's start with a simple refactoring. Here's some code from early on in fightRound:

if hero.weaponType == 0:
    hero.swing = hero.AGI * 3
elif hero.weaponType == 1:
    hero.swing = hero.AGI * 2
elif hero.weaponType > 1:
    hero.swing = hero.AGI

[...]

if enemy.weaponType == 0:
    enemy.swing = enemy.AGI * 3
elif enemy.weaponType == 1:
    enemy.swing = enemy.AGI * 2
elif enemy.weaponType > 1:
    enemy.swing = enemy.AGI


These two blocks are the same, except the first one references hero everywhere and the second references enemy. As a first step, I would like to see this become something like:

hero.swing = getWeaponSwing(hero)
enemy.swing = getWeaponSwing(enemy)


where getWeaponSwing(char) was a simple implementation of that common code.

Similarly the interleaved code that calculates attacks and misses is also repeated. This could become its own function, or perhaps merged with the suggested getWeaponSwing and given a more inclusive name.

Refactoring into data

A little further down, just past the comment # Just until I get items in..., there is an abbreviated if/elif tree that sets mindmg and maxdmg per the type of weapon. Let's consider what refactoring can do here. If the overall structure looks the same for all the omitted weaponType values, consider a data-driven refactoring:

# up above, probably globally:
WeaponDamageCoef = [
    (10, 10), # level 0
    (5, 25),  # level 1
    ... ]

# back in fightRound
coef = WeaponDamageCoef[hero.weaponType]
hero.mindmg = coef[0] * hero.LVL
hero.maxdmg = coef[1] * hero.LVL

# or, a more advanced way
hero.mindmg, hero.maxdmg = [coef * hero.LVL for coef in WeaponDamageCoef[hero.weaponType]]


Refactoring into classes

It's possible that the above scenario was simplified. Maybe it's not always a coefficient you can look up by weapon type, or it's not always multiplied by the level. If you need further customization, you can make multiple weapon classes that offer the same interface. Then store an instance of the weapon on your hero and enemy, and let the weapon figure out its thing. Here's a roughed out example of that:

class Weapon(object):
    def getDamageRange(self, char):
        return 10 * char.LVL, 10 * char.LVL

class MagicWeapon(object):     # or perhaps inherit from Weapon, or some shared base
    def getDamageRange(self, char):
        return 5 * char.LVL + 3 * char.INT, 7 * char.LVL + 8 * char.INT
...
hero.weapon = MagicWeapon()
...
hero.mindmg, hero.maxdmg = hero.weapon.getDamageRange(hero)


Next steps

Look for as many instances of repeating code as you can find, and try to refactor them into helpers. After you do this, the code should become smaller and easier to manage. You may find that after the first level of refactoring, other similarities start to become apparent and you can do further higher level refactorings.

There are a lot of other opportunities to improve this code. Refactoring won't fix them all. My hope is that once you reduce the quantity of code through good factoring, it will be easier to address the opportunities that remain.

Code Snippets

if hero.weaponType == 0:
    hero.swing = hero.AGI * 3
elif hero.weaponType == 1:
    hero.swing = hero.AGI * 2
elif hero.weaponType > 1:
    hero.swing = hero.AGI

[...]

if enemy.weaponType == 0:
    enemy.swing = enemy.AGI * 3
elif enemy.weaponType == 1:
    enemy.swing = enemy.AGI * 2
elif enemy.weaponType > 1:
    enemy.swing = enemy.AGI
hero.swing = getWeaponSwing(hero)
enemy.swing = getWeaponSwing(enemy)
# up above, probably globally:
WeaponDamageCoef = [
    (10, 10), # level 0
    (5, 25),  # level 1
    ... ]

# back in fightRound
coef = WeaponDamageCoef[hero.weaponType]
hero.mindmg = coef[0] * hero.LVL
hero.maxdmg = coef[1] * hero.LVL

# or, a more advanced way
hero.mindmg, hero.maxdmg = [coef * hero.LVL for coef in WeaponDamageCoef[hero.weaponType]]
class Weapon(object):
    def getDamageRange(self, char):
        return 10 * char.LVL, 10 * char.LVL

class MagicWeapon(object):     # or perhaps inherit from Weapon, or some shared base
    def getDamageRange(self, char):
        return 5 * char.LVL + 3 * char.INT, 7 * char.LVL + 8 * char.INT
...
hero.weapon = MagicWeapon()
...
hero.mindmg, hero.maxdmg = hero.weapon.getDamageRange(hero)

Context

StackExchange Code Review Q#41264, answer score: 7

Revisions (0)

No revisions yet.