Skip to content
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

Merged
merged 46 commits into from Jun 12, 2018
Merged

contrib: Magic, items systems for 'turnbattle' #1518

merged 46 commits into from Jun 12, 2018

Conversation

FlutterSprite
Copy link
Contributor

@FlutterSprite FlutterSprite commented Nov 15, 2017

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.

@aliceafterall
Copy link
Member

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

@FlutterSprite
Copy link
Contributor Author

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.

@FlutterSprite FlutterSprite changed the title contrib: Magic system for 'turnbattle' contrib: Magic, items systems for 'turnbattle' Nov 18, 2017
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!
@FlutterSprite
Copy link
Contributor Author

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.
@FlutterSprite
Copy link
Contributor Author

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.

@FlutterSprite
Copy link
Contributor Author

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!

@FlutterSprite
Copy link
Contributor Author

All right, unit tests for the magic system stuff are in too!

@Griatch
Copy link
Member

Griatch commented Dec 12, 2017

@FlutterSprite Good job! I'll look this over soon, but it may take a few days (these PRs are pretty big after all!)

@FlutterSprite
Copy link
Contributor Author

Thank you so much!! I'm hoping I can have these all nice and ready for Evennia 0.8!

Copy link
Member

@Griatch Griatch left a 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.


# Test magic commands
def test_turnbattlemagiccmd(self):
self.call(tb_magic.CmdStatus(), "", "You have 100 / 100 HP and 20 / 20 MP.")
Copy link
Member

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?

Copy link
Contributor Author

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.


# Test functions in tb_items.
def test_tbitemsfunc(self):
attacker = create_object(tb_items.TBItemsCharacterTest, key="Attacker")
Copy link
Member

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.

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):
Copy link
Member

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.

user.db.conditions = {"Poisoned":[5, user]}
tb_items.itemfunc_cure_condition(test_healpotion, user, user)
self.assertTrue(user.db.conditions == {})
# Delete the test character
Copy link
Member

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
Copy link
Member

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.
Copy link
Member

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.

Copy link
Contributor Author

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.
Copy link
Member

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
"""
Copy link
Member

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)
"""
Copy link
Member

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.
Copy link
Member

@Griatch Griatch Dec 18, 2017

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.

@Griatch
Copy link
Member

Griatch commented Jan 20, 2018

@FlutterSprite Any further progress on this?

@FlutterSprite
Copy link
Contributor Author

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.

@Griatch
Copy link
Member

Griatch commented Jan 20, 2018

@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!

@Griatch
Copy link
Member

Griatch commented Mar 10, 2018

@FlutterSprite Gotten any more time to wrap this one up?

@FlutterSprite
Copy link
Contributor Author

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!

@Griatch
Copy link
Member

Griatch commented Apr 21, 2018

@FlutterSprite Bump.

@FlutterSprite
Copy link
Contributor Author

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!

@FlutterSprite
Copy link
Contributor Author

Hoping to finally resolve all this stuff today!

@FlutterSprite
Copy link
Contributor Author

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!

@Griatch
Copy link
Member

Griatch commented Jun 12, 2018

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.

@Griatch Griatch merged commit d550f3e into evennia:develop Jun 12, 2018
@Griatch
Copy link
Member

Griatch commented Jun 12, 2018

There - patched out the tickerhandler in the unit test. Merged now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants