HabitRPG / habitica

A habit tracker app which treats your goals like a Role Playing Game.
https://habitica.com
Other
11.95k stars 4.08k forks source link

Make Ethereal Surge work only on non-Mages in the party #8860

Closed SabreCat closed 6 years ago

SabreCat commented 7 years ago

Description

Intrepid gamebreakers have noticed it's possible to use multiple Mages in a party to cast skills indefinitely without checking off any tasks or crossing from one day to the next. They can give the rest of the party enough Mana to cast their skills without limits, increase INTelligence to stratospheric limits, etc. All it takes is Mages using Ethereal Surge to give one another Mana. The Surge doesn't raise one's own Mana, but it does increase the Mana of other Mages who can then use Ethereal Surge to give back the Mana you spent, and so on!

Normally we don't care overmuch when people discover "cheats" in Habitica, as the core concept of checking off real-life tasks to earn in-game rewards is something that only works on an honor system to begin with. As folks in the Tavern typically put it, "the only person you're cheating is yourself." For this exploit, however, it makes it a bit too simple to invalidate the rest of Habitica's game elements: damage never matters if you can cast unbounded Blessings, you never have to check off a task to attack a monster if you can Brutal Smash and Burst of Flames indefinitely, and so on. We've even ended up with server problems due to people setting up automated scripts to chain-cast Ethereal Surge.

One simple tweak should make infinite Ethereal Surge loops impossible: make Ethereal Surge affect only non-Mages. You can give up your Mana to help party members of other classes use their own skills, but you can't use Ethereal Surge to create a perpetual motion machine among a group of Mages.

LunarGem commented 7 years ago

I could try this! I'd just need some time to familiarize myself with the codebase.

SabreCat commented 7 years ago

@LunarGem: Great, go for it! I suspect there is a simple modification to be made here that'll do the trick. https://github.com/HabitRPG/habitica/blob/develop/website/common/script/content/spells.js#L52-L66

LunarGem commented 7 years ago

@SabreCat I think I have the solution, I just need to find the code that stores the data into that "member" variable. Do you know where that is? Either that or to find code that allows fetching data from UID.

SabreCat commented 7 years ago

member is the iteratee of target in the each call (see https://lodash.com/docs/4.17.4#forEach). So member means a member of the party, and the spell's effects will be applied to each member of the party in turn. The array of party members is sent from the client here: https://github.com/HabitRPG/habitica/blob/develop/website/client-old/js/controllers/rootCtrl.js#L364-L375

EDIT -- Minor correction: the list of spell targets is put together there, but it's actually sent to the server a little further down in castEnd.

LunarGem commented 7 years ago

Ah, alright. This fix should be pretty simple, but I'll need to learn me some LoDash first. πŸ˜›

librarianmage commented 7 years ago

What should happen if all members in the party are Mages?

LunarGem commented 7 years ago

@SabreCat @MathWhiz Hmm. Perhaps the limitation should be one Ethereal Surge per cron? Or if Mage has cast Ethereal Surge this cron, you can't cast Ethereal Surge on them? I notice this fix is exploding in complexity... πŸ˜›

SabreCat commented 7 years ago

@MathWhiz: Good question! Perhaps it should just error out with "no legal targets". But let me check in with the rest of the team on that.

SabreCat commented 7 years ago

Confirmed with @lemoness, saying "No valid targets" (or similar) and refunding the MP spent would do the trick in that scenario!

LunarGem commented 7 years ago

Got it, I'll definitely do that when I do the fix. Having some trouble installing things atm.

LunarGem commented 7 years ago

Are there some updated links for the new code that I should be looking at now?

SabreCat commented 7 years ago

@LunarGem: The logic for the skill behavior still lives in the same place in the common code, above. I don't know that anything will need to change client-side, but if so, https://github.com/HabitRPG/habitica/blob/develop/website/client/components/tasks/spells.vue is a place to start!

LunarGem commented 7 years ago

I had another client side skills related assignment anyway, thanks!

LunarGem commented 7 years ago

The link you gave me to help me understand the member array doesn't work now, is there a new version of that? @SabreCat

SabreCat commented 7 years ago

Ah! Right. This is where that sort of thing hangs out now: https://github.com/HabitRPG/habitica/blob/develop/website/client/mixins/spells.js#L26

LunarGem commented 7 years ago

Thanks! I think when I find how to determine the class from the member array my fix will be complete. @SabreCat

LunarGem commented 7 years ago

@SabreCat Do we have a method we can use (via the api or otherwise) to get the class of a given user ID? EDIT: nvm, found what I was looking for! I understand the code a lot more today than I did yesterday πŸ˜›

LunarGem commented 7 years ago

Last question, how do I send the red notification bubble that will have the error in it? @SabreCat

LunarGem commented 7 years ago

@SabreCat Ping? How do the snackbars work? @Alys ?

SabreCat commented 7 years ago

Ah, sorry @LunarGem! I actually did a bit of a linkdump on another issue ticket regarding how the snackbars work and where errors are handled. https://github.com/HabitRPG/habitica/issues/9213#issuecomment-337688371 You may be OK just adding an error response on the API side, and once that issue is fixed it'll display on the client.

LunarGem commented 7 years ago

So to throw an error, should I use a try/catch or try to call something with the axios thing?

SabreCat commented 7 years ago

@LunarGem I'd say to throw an error either from the API or in the common code, whatever ends up being most feasible.

https://github.com/HabitRPG/habitica/blob/develop/website/common/script/content/spells.js#L62 - common code (shared client/server) for the Ethereal Surge spell, could return early with an error https://github.com/HabitRPG/habitica/blob/develop/website/server/controllers/api-v3/user.js#L584-L587 - various error conditions in the API call, might be able to add one there

LunarGem commented 7 years ago

I've finished the fix! My local install is complaining at the moment, should I submit the pull now or wait until I've wrangled the server?

Alys commented 7 years ago

@LunarGem If you've fully tested it manually in a local install's website and it works but you can't get the automated tests to run, then submit a PR and check the travis-ci build's test results when they appear. Otherwise, it's best to get your local install working so you can be sure of the fix before you submit it.

LunarGem commented 6 years ago

Any tips on how to make the line endings come out right?

librarianmage commented 6 years ago

What editor are you using? On Tue, Oct 31, 2017 at 2:17 PM LunarGem notifications@github.com wrote:

Any tips on how to make the line endings come out right?

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HabitRPG/habitica/issues/8860#issuecomment-340877497, or mute the thread https://github.com/notifications/unsubscribe-auth/ALz_8zGCS8u_loNTEFpQcUDl7luFU5TXks5sx3IwgaJpZM4OTUuy .

SabreCat commented 6 years ago

@LunarGem The answer will probably vary based on your dev environment. What OS are you on, and what are you using to make your edits?

LunarGem commented 6 years ago

Windows 10 v1709 using the most recent version of Atom. @SabreCat @MathWhiz

LunarGem commented 6 years ago

I'm also getting this error on the npm start part of my local install: Error: Cannot find module 'C:\Users\<me>\Documents\GitHub\habitica\node_modules\bcrypt\lib\binding\bcrypt_lib.node' @SabreCat Know what I should do about that? I've refreshed every file in my repo a bunch of times.

librarianmage commented 6 years ago

Have you run rm -rf node_modules/ and npm start On Tue, Oct 31, 2017 at 3:03 PM LunarGem notifications@github.com wrote:

I'm also getting this error on the npm start part of my local install: Error: Cannot find module 'C:\Users\Lauren\Documents\GitHub\habitica\node_modules\bcrypt\lib\binding\bcrypt_lib.node' @SabreCat https://github.com/sabrecat Know what I should do about that? I've refreshed every file in my repo a bunch of times.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HabitRPG/habitica/issues/8860#issuecomment-340889944, or mute the thread https://github.com/notifications/unsubscribe-auth/ALz_8-ocG6m14BtHEFgL_Q6_n2CrsByVks5sx30RgaJpZM4OTUuy .

LunarGem commented 6 years ago

@MathWhiz I'll try that next.

librarianmage commented 6 years ago

Sorry, npm install. On Tue, Oct 31, 2017 at 5:53 PM LunarGem notifications@github.com wrote:

@MathWhiz https://github.com/mathwhiz I'll try that next.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HabitRPG/habitica/issues/8860#issuecomment-340930866, or mute the thread https://github.com/notifications/unsubscribe-auth/ALz_8xrtA84rJlSI-cCTrovpBSD8YUfCks5sx6TNgaJpZM4OTUuy .

LunarGem commented 6 years ago

Thanks, @MathWhiz , my install works now! I changed the way I was doing the error, making it more like the errors when Chilling Frost has already been cast for the day, etc. I'm hoping I've done that correctly, maybe I could submit the pull and someone could check it? I'd mark it WIP if that helps. @SabreCat

SabreCat commented 6 years ago

@LunarGem Absolutely, go ahead!

LunarGem commented 6 years ago

It's here: https://github.com/HabitRPG/habitica/pull/9385

LunarGem commented 6 years ago

Anybody got a chance to check that PR yet?

paglias commented 6 years ago

@LunarGem can you fix this https://github.com/HabitRPG/habitica/pull/9385/files#diff-6cabb9f4dba702cc59d4717fb26ec465R3?

LunarGem commented 6 years ago

Gosh, that would have been fixed ages ago if I'd remembered we were waiting on me!