WIP: Implement Basic Stats #9
Conversation
This reverts commit 9ce7935.
| @@ -10,16 +9,39 @@ function Entity (entity) { | |||
| this.type = entity.type || 'Unknown'; | |||
| this.name = entity.name || this.id; | |||
|
|
|||
| this.experience = 0; | |||
| this.EXP = 0; | |||
There was a problem hiding this comment.
Avoid changing the name of properties on templates, since other parts of the program most likely depend on them. Also, as a general rule, try to keep all properties human-friendly — there's no need to shorten variable names in modern languages!
There was a problem hiding this comment.
Got it, I'll change it back to this.experience
|
|
||
| this.equipment = { |
There was a problem hiding this comment.
Scanned over the rest of the file; is equipment just being removed? Same feedback as earlier comment.
There was a problem hiding this comment.
Didn't intend to remove equipment completely, that may have been something I was trying to accomplish but meant to undo.
| shielding: 0, | ||
| smarts: 0, | ||
| willpower: 0, | ||
| charm: 0 |
There was a problem hiding this comment.
Whoa, that's a lot of different stats! Let's coordinate out-of-band to go over the player experience.
There was a problem hiding this comment.
There's potential for slimming the amount down. Or at least subdividing them to make them reader-friendly (original subdivisions were physical, magical, and mental).
| summonCards: 0, | ||
| spellCards: 0, | ||
| enchantCards: 0, | ||
| leylineCards: 0 |
There was a problem hiding this comment.
These two new properties seem like they should be simplified and variable names thought about a bit more.
There was a problem hiding this comment.
Those are meant to be "maximum" variables, so the names should likely be changed to represent this, or they should be condensed -- or maximums removed entirely. Not sure.
| @@ -29,29 +51,66 @@ function Entity (entity) { | |||
| return this; | |||
| } | |||
|
|
|||
| Object.defineProperty(Entity.prototype, 'level', { | |||
There was a problem hiding this comment.
This has to stick around, as it's used elsewhere.
|
|
||
| return Entity.stats.attack; | ||
| } | ||
| }); |
There was a problem hiding this comment.
You don't need to use Object.defineProperty() if the value isn't calculated on the fly. Just do something like this:
| }); | |
| this.attack = `foo`; |
If you're just looking for the raw statistic, you can remove these entirely.
|
I've reviewed everything here to date — nice work, dude! Let's chat out-of-band to begin next steps. 🚀 |
This is a list of changes made while I was messing around with IdleRPG in a VM. entity.js is the one with the most change, as there was an attempt to implement stats from the below document as a test.
https://docs.google.com/document/d/1oaVJZnrUKijAUTkEgU7HETitG3MPeXecwQm31HmGzJo/edit?usp=sharing