WIP fix: change rolling #291
Conversation
src/module/config.ts
Outdated
| @@ -112,13 +112,10 @@ const DIFFICULTIES = Object.freeze({ | |||
| } | |||
| }); | |||
|
|
|||
| const CRIT = Object.freeze({"SUCCESS": 1, "FAIL": 2}); | |||
There was a problem hiding this comment.
This is simplified moved to twodsix.d.ts and simplified to a proper enum
src/module/entities/TwodsixItem.ts
Outdated
| return diceRoll; | ||
| } | ||
|
|
||
| public async rollDamage(rollMode:string, bonusDamage="", showInChat=true):Promise<Roll> { |
There was a problem hiding this comment.
this method is mostly unchanged. Some small changes.
| @@ -56,8 +56,14 @@ export class TwodsixActorSheet extends AbstractTwodsixActorSheet { | |||
| super.activateListeners(html); | |||
|
|
|||
| // Rollable abilities. Really should be in base class, but that will have to wait for issue 86 | |||
|
|
|||
| html.find('.rollable').on('click', (this._onRoll.bind(this))); | |||
There was a problem hiding this comment.
Here I kept the old way of making a throw. So that it's possible to compare to the old way
| // somewhat worth it. | ||
| let skill: TwodsixItem; | ||
| try { | ||
| skill = TwodsixItem.createOwned(data, this.actor) as TwodsixItem; |
There was a problem hiding this comment.
Yeah, as the comment above says, this feels quite hacky. There are other ways to solve this, but the rest of the code becomes very nice by creating a temporary skill like this.
src/module/utils/TwodsixDiceRoll.ts
Outdated
| this.roll.roll(); | ||
| } | ||
|
|
||
| constructor (settings: TwodsixRollSettings, actor: TwodsixActor, skill: TwodsixItem=null, item: TwodsixItem=null) { |
There was a problem hiding this comment.
Ops. Should be moved to the top of the class
src/module/utils/TwodsixDiceRoll.ts
Outdated
| import { getKeyByValue } from "./sheetUtils"; | ||
| import { TwodsixRollSettings } from "./TwodsixRollSettings"; | ||
|
|
||
| export class TwodsixDiceRoll { |
There was a problem hiding this comment.
This class holds a twodsix skill/characteristic roll object, which is aware about effect, target, etc. It also can send a chat message about the result.
|
This pull request is now in conflicts. Could you fix it? 🙏 |
| import { getKeyByValue } from "./sheetUtils"; | ||
|
|
||
|
|
||
| export class TwodsixRollSettings { |
There was a problem hiding this comment.
Roll settings. It has a set of default settings which it will apply if it doesn't receive any specific settings. Can also ask for specifics through the dialog
jonepatr
force-pushed
the
issue-212
branch
5 times, most recently
from
f4dbdc5 to
35abab1
Compare
Big change to how rolling is performed. Backward compatible, and using both methods for now. Not meant to be merged. Fix xdy#212
change so that newItem flag is now set in the TwodsixItem.create instead of setting it through a hook. It's a bit cleaner to do this way.
|
Jag tar och plockar in detta till pajon_refactor, ser bra ut. |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Big change to how rolling is performed. Backward compatible, and using
both methods for now. Not meant to be merged.
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
hopefully not much :) I removed the dropdown to select characteristics when rolling for a single characteristics. Other than that behavior should be the same
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No
Other information:
Fix Add support for ammunition and other consumables #212