Expansion of 'turnbattle' system contrib: tb_range #1478
Conversation
Now that this system has been moved to /turnbattle/, this file is obsolete.
Adds a system for range and movement to the 'turnbattle' contrib. This is based on the abstract movement and positioning system I made for 'The World of Cool Battles', my learning project - fighters' absolute positions are not tracked, only their relative distance to each other and other objects. Commands for movement as well as distinction between melee and ranged attacks are included.
Adds information about the tb_range module to the readme.
|
The unit tests aren't in yet, but I'd like to know what everyone thinks! This contrib's a little more complex than my others. |
|
|
||
|
|
||
| def roll_init(character): | ||
| """ |
There was a problem hiding this comment.
Very nice with these heavily documented "easily-replaceable" functions. :)
| attacker.location.msg_contents("%s hits %s with a %s attack for %i damage!" % (attacker, defender, attack_type, damage_value)) | ||
| apply_damage(defender, damage_value) | ||
| # If defender HP is reduced to 0 or less, announce defeat. | ||
| if defender.db.hp <= 0: |
There was a problem hiding this comment.
I understand this whole function is meant to be over-ridden per game, but you have separate functions for nearly all components of the combat. Would it not be an idea to have a stub at_defeat(winner, loser) or similar for handling whatever happens and the echoes needed? Not a request really, just a thought. :)
There was a problem hiding this comment.
This would be a good thing to separate out - I'll have to apply this change across all the 'turnbattle' modules.
| mover (obj): The object moving | ||
| target (obj): The object to be moved toward | ||
| """ | ||
| mover.db.Combat_Range[target] -= 1 |
There was a problem hiding this comment.
Overall, PEP8 standards (lower case variable names) would be a good idea to keep also for Attributes, since they look just like normal properties. So combat_range rather than Combat_Range and so on for the various Attributes. Internally they are case-insensitive, so doing so won't affect existing data, but only how they show in-code.
There was a problem hiding this comment.
This would be an easy change! I'll apply this across the rest of the modules too.
| if defender.db.hp <= 0: | ||
| attacker.location.msg_contents("%s has been defeated!" % defender) | ||
|
|
||
| def distance_dec(mover, target): |
There was a problem hiding this comment.
From how this is used in-code, distance_dec and distance_inc are helper functions to approach and withdraw, the latter being the functions intended to be used externally. If this is the case you should clarify which functions are part of your API and which are internal-use only. This can be done either by marking these helper-functions as private (_distance_dec/inc) or by embedding them inside their main functions.
There was a problem hiding this comment.
Ah! This was something I knew nothing about - I'll definitely address this. There's not really anywhere else I would think to use distance_dec and distance_inc by themselves.
| target.db.Combat_Range[mover] = 0 | ||
| mover.db.Combat_Range = target.db.Combat_Range | ||
| # Assure everything else has the same distance from the mover and target, now that they're together | ||
| for object in mover.location.contents: |
There was a problem hiding this comment.
object is a reserved Python word. Never use a reserved word as a variable name even if you get no error from it here - not only is it a potential source of insidious bugs, it's also against PEP8 and Python praxis and will also trigger all sorts of warnings in external style checkers down the line. Use something like obj instead.
There was a problem hiding this comment.
Whoops!! I'll definitely fix this.
| Returns: | ||
| (bool): True if in combat or False if not in combat | ||
| """ | ||
| if character.db.Combat_TurnHandler: |
There was a problem hiding this comment.
This can simply and more succinctly be expressed as return bool(character.db.Combat_TurnHandler).
| turnhandler = character.db.Combat_TurnHandler | ||
| currentchar = turnhandler.db.fighters[turnhandler.db.turn] | ||
| if character == currentchar: | ||
| return True |
| if is_in_combat(dropper) and not is_turn(dropper): | ||
| dropper.msg("You can only drop things on your turn!") | ||
| return False | ||
| return True |
There was a problem hiding this comment.
Minor PEP8 nag: Put one empty line between all methods in a class.
| """ | ||
|
|
||
|
|
||
| class TBRangeTurnHandler(DefaultScript): |
There was a problem hiding this comment.
I am a little divided on placing this important component at the end of the module. The handler is used and referenced throughout the other components, so when reading the code one wonders what it is and where it comes from. For this reason I always push for a module to be ordered so that things are defined above the code that use them. Hence I feel this should be defined higher up, or even at the top.
There was a problem hiding this comment.
I think it was my thought originally that the turn handler used lots of the other functions, it should come after them, but at some point I moved many of those functions into the turn handler itself. I'll put it higher up here and in the rest of the modules.
| if self.db.turn > len(self.db.fighters) - 1: | ||
| self.db.turn = 0 # Go back to the first in the turn order once you reach the end. | ||
| newchar = self.db.fighters[self.db.turn] # Note the new character | ||
| self.db.timer = 30 + self.time_until_next_repeat() # Reset the timer. |
There was a problem hiding this comment.
Timeouts like these seem very game-specific to hide away inside the Script body like this. Might it not be an idea to have a block of global constants up top, like TURN_TIMEOUT = 30 for things like this? Would also make it easier for the dev to see which knobs are easy to tweak ...
There was a problem hiding this comment.
This is definitely a good idea - actions per turn would be another good variable to be able to easily mess with. I think it was my original thought that adding extra flexibility with variables would complicate things too much for a system that's mostly meant to be completely overhauled, but some things like this would be universal enough that I can't imagine they'd be taken out completely!
This makes the code a bit more readable and fixes a bug in withdrawing that didn't take other objects into account properly.
|
There! Still no unit tests, but I addressed all the concerns you raised. This involved going back and making changes in tb_basic and tb_equip as well. |
I almost forgot - distance_inc is actually used by both 'approach' and 'withdraw', since approaching an object might put you farther away from others. So, I moved it back to its own function.
|
Cool! I'll review again when the unit tests are in then. :) |
|
The unit tests are in! This one was quite a pain. |
This was an attempt to try to fix some strange 'unhandled error in deffered' results while unit testing the contrib folder. It didn't work, but it's probably good to do anyway.
|
@FlutterSprite This looks good I think. There is one thing I'm missing here though, when reading the contrib's documentation, and that is how decoupled these various modules are. That is, when I look at the |
|
Gotcha! I'll clarify things in the readme real quick. |
|
How's that? |
|
Thanks, merged now! One thing to remember for the next time - for your own sanity of mind, don't develop things like this in your own copy of the upstream develop (or master) branch. It's better that you keep your version of develop in sync with upstream and do your development in a new branch off that (this you can then just delete once your contributions is available upstream). Otherwise, if I should for example rebase or squash your PR's commits, your version of devel/master branch will be different from the upstream one and you'll face merge conflicts. |
|
Oh, that makes a lot of sense! Slowly but surely, I'm learning how to use GitHub properly. This is the first time I've actually committed my changes from the command line instead of pasting them into the website like a scrub. |
Adds a new contrib module: tb_range, an expansion for the turnbattle contrib which implements a system of movement and positioning.
Brief overview of PR changes/additions
The tb_range module includes an abstract system of movement and position in combat, which works by tracking different fighters and objects' relative distance to each other without tracking their absolute position. This allows for a simple and intuitive (if imprecise) system for movement in a text-based game that doesn't disadvantage players on screen readers or require navigating around a grid.
Motivation for adding to Evennia
I plan to add a few more expansions to the 'turnbattle' contrib folder demonstrating various expansions to the basic combat systems, and then a 'complete' version which implements all of the subsystems of the various modules and implements interactions between them (like melee and ranged weapons for the equipment system in tb_equip, for example). Ultimately, the goal is to have a fully-featured combat system ready for aspiring MUD creators who want to get their game off the ground right away, and to give many options to let them use as much or as little of it as they like.
Other info (issues closed, discussion etc)
This system, like the 'turnbattle' contrib itself, is based around its implementation in my learning project "The World of Cool Battles", though it has been streamlined in some places (fewer possible range values and lack of multi-step movement) and expanded in others (the tracking of distances to objects and exits).