New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
contrib: Magic, items systems for 'turnbattle' #1518
Conversation
Also added more spell verification in the 'cast' command, accounting for spell's MP cost and whether it can be used in combat
I am super excited about this! Ainneve has had an issue open for a long time about adding a magic system, and I'm hoping we can replace Ainneve's combat system with Turnbattle! I'm gonna read over this, and see if I have any questions! :D |
Merge develop
Merge magic and items branches
Since my system for items and conditions is almost ready to go too, I decided to merge that in with this one! I'll update the PR's description to reflect this. |
Added the ability for attack items to inflict conditions on hit, as well as items that can cure specific conditions.
at_update() erroneously changed the turnchar on conditions during combat - this has been fixed.
Turns out 1 == True, but not 1 is True - learn something new every day!
All right, this is ready to get some eyes on it, I think! I'll get started on the unit tests once everything looks good! |
Whoops! I forgot that ALL my test characters are getting subscribed to the ticker handler here - maybe that's the problem?
Travis won't even tell me why it failed this time.
I just commented out the entire section for unit tests for tb_items. I'll get started on tests for tb_magic soon and come back to this later. |
I did it! I found a way to make everything build nice - I just made another version of the character that didn't use the TickerHandler, and it was smooth sailing from there! I'll finish the rest of the unit tests soon! |
All right, unit tests for the magic system stuff are in too! |
@FlutterSprite Good job! I'll look this over soon, but it may take a few days (these PRs are pretty big after all!) |
Thank you so much!! I'm hoping I can have these all nice and ready for Evennia 0.8! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, this is a neat and well presented package I think. I have only a few points of comment and feedback to look over.
evennia/contrib/tests.py
Outdated
|
||
# Test magic commands | ||
def test_turnbattlemagiccmd(self): | ||
self.call(tb_magic.CmdStatus(), "", "You have 100 / 100 HP and 20 / 20 MP.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These all appear to be failure-state tests. Are these just intended to test that the commands exist without causing immediate syntax errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, yes. This is how it is for all the 'turnbattle' modules - I think going through and cleaning them up and making the tests more thorough and consistent might be its own project, part of making all the 'turnbattle' modules more streamlined.
evennia/contrib/tests.py
Outdated
|
||
# Test functions in tb_items. | ||
def test_tbitemsfunc(self): | ||
attacker = create_object(tb_items.TBItemsCharacterTest, key="Attacker") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These creation-lines appear to be boiler-plate code. Put it in a setUp
method of the tester class so you don't have to put this at the beginning of each test. You can also have a tearDown
method if you want to do any house cleaning after each test.
evennia/contrib/tests.py
Outdated
self.call(tb_magic.CmdPass(), "", "You can only do that in combat. (see: help fight)") | ||
self.call(tb_magic.CmdDisengage(), "", "You can only do that in combat. (see: help fight)") | ||
self.call(tb_magic.CmdRest(), "", "Char rests to recover HP and MP.") | ||
|
||
|
||
class TestTurnBattleFunc(EvenniaTest): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future reference, unit tests are better split into more methods than this; the magic and items would be better as separate classes with multiple methods testing the different pieces of functionality that you mention in your comments. That way you can easily (re)run individual tests (you can run individual tests down to the method level) if one fails without having to rerun all of them.
evennia/contrib/tests.py
Outdated
user.db.conditions = {"Poisoned":[5, user]} | ||
tb_items.itemfunc_cure_condition(test_healpotion, user, user) | ||
self.assertTrue(user.db.conditions == {}) | ||
# Delete the test character |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of cleanup belongs in a tearDown
method, it will then be called after automatically after each test-method in the class.
---------------------------------------------------------------------------- | ||
""" | ||
|
||
TURN_TIMEOUT = 30 # Time before turns automatically end, in seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constants up here makes it much cleaner. :)
|
||
These functions carry out the action of using an item - every item should | ||
contain a db entry "item_func", with its value being a string that is | ||
matched to one of these functions in the ITEMFUNCS dictionary below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this way of mapping is fine, a more Evennia-esque way would be to have these functions in a separate module and use the callables_from_module
Evennia utility - it returns a dictionary keyed by their def
names in the module, with values being the callables. That way you could easily have people add new stuff by just having them add the path to a new module with item-funcs to some setting (see for example how lockfuncs, inlinefuncs etc work). Not necessary but probably more consistent with how other similar function-plugins work in Evennia.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good! I'll keep that in mind - I'm interested in going through all the turnbattle modules and kind of cleaning them up a bit at some point.
|
||
This module is meant to be heavily expanded on, so you may want to copy it | ||
to your game's 'world' folder and modify it there rather than importing it | ||
in your game and using it as-is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good docstring here!
self.db.spells_known = [] # Set empty spells known list | ||
self.db.max_mp = 20 # Set maximum MP to 20 | ||
self.db.mp = self.db.max_mp # Set current MP to maximum | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this up with the normal docstring of this method. Use a Notes:
section if you want to separate it from the header.
spelldata["spellfunc"](caller, spell_to_cast, spell_targets, spelldata["cost"], **kwargs) | ||
except Exception: | ||
log_trace("Error in callback for spell: %s." % spell_to_cast) | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be moved to the end of the main docstring or made into a comment with #
.
self.caller.db.mp = self.caller.db.max_mp # Set current MP to maximum | ||
self.caller.location.msg_contents("%s rests to recover HP and MP." % self.caller) | ||
""" | ||
You'll probably want to replace this with your own system for recovering HP and MP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this type of commenting is inconsistent with anything else in Evennia, please make this into a comment with #
or move it together with the regular doc string.
@FlutterSprite Any further progress on this? |
Sorry! I've been really busy with my main project (which just got announced! Check it out! http://www.grappleforce.com ) so I haven't really had any time to go back to Evennia stuff lately. Once things slow down for me a bit, I'll get back to getting this squared away. |
@FlutterSprite Oh, I remember you showing me some animations from this before! That grappling-hook thingie remains very cool and I like the artwork and details of this. Very cool. :) Glad things are progressing! |
@FlutterSprite Gotten any more time to wrap this one up? |
Sorry! My main project still is eating up most of my time and attention. I re-read through your comments and I may be able to make some of the more minor tweaks and syntax things soon so we can get everything merged - I don't think I'll have any time for any major overhauls to how this is all put together (especially the stuff with unit tests, which I still have only the barest familiarity with). If there's anyone else interested in helping get it finished up, I'd really appreciate it! |
@FlutterSprite Bump. |
I'm probably going to take a few days or a week off from my main project sometime soon, so I'll get back to this during that time and hopefully get it closed out! |
Hoping to finally resolve all this stuff today! |
All right, I split the Turnbattle modules' unit test classes up more, and refactored stuff into setUp and tearDown methods, so it should all be a lot cleaner and more proper now. This should address everything brought up before. I really hope this is enough to get it all merged in! |
There are a few comments that needs to be kept in mind, but I don't think there is a need to hold off on merging this. :) The main issue I have with this as it stands however is that it appears the unit tests are not isolated - probably due to the tickerhandler components of these tests (dirty reactor), it clashes with other tests and fails the test suite. |
There - patched out the tickerhandler in the unit test. Merged now :) |
Brief overview of PR changes/additions
Adds another version of the turn based combat system that includes a system for casting magic spells or using similar special abilities - spells are defined in a dictionary that determines their cost, valid spell targets, and the circumstances under which the spell can be cast. Spell names are matched to functions, and the spell definitions can pass kwargs to them, meaning the same function can be re-used to make multiple spells.
This also adds another module for usable items and status effects - items work similarly to spells, executing arbitrary functions, and have some options for limited uses, consuming spent items, and replacing spent items with another object as 'residue' (such as a glass bottle being left behind after using a potion). This module also includes conditions, temporary effects on characters that can be beneficial or harmful, with set durations that count down each turn or every 30 seconds when out of combat.
Motivation for adding to Evennia
Magic systems are a fairly common element of MUDs, so a ready-to-go system for spellcasting is something that may be useful to a lot of developers - it also serves as a good example of a few code techniques that may not be immediately obvious to Python beginners, like storing references to functions and passing arbitrary kwargs.
Similarly, items and status effects are a big part of RPGs in general and MUDs in particular - the items system is very flexible, and can be used for a wide variety of systems and tasks.
Other info (issues closed, discussion etc)
The unit tests aren't in yet - this one's going to be a doozy to test, I think - but it's ready to get some eyes on it! After this, I'm going to make one more expansion (items and status effects) before combining them all into a complete system.
UPDATE: I've merged in my items/status effects system. This one isn't quite ready to go yet, as the documentation and examples aren't done yet, but most of the base functionality is in and working properly.