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

Add weapon features' toggles and integrate features with chat and combat actions #47

Merged
merged 6 commits into from
Mar 10, 2021

Conversation

giant-teapot
Copy link
Collaborator

I didn't get as much time as I wanted to work on this, but finally: here it is. You can now specify weapon features in a way that is easier for the system to understand.

The bulk of the change can be summarized by the datamodel change:

// Before:
"features": "something",

// After:
"features": {
    "edged": false,
    "pointed": false,
    "blunt": false,
    "parrying": false,
    "hook": false,
    "slowReload": false,
    "others": "something else"
},

The rest is just making it visible and having a few quality of life improvements. So what do we have in stock?

  • Features can be checked/unchecked in the weapon sheet.
  • Features can be translated.
  • Features now appear in the weapon chat link (translated if possible).
  • Combat actions in the character sheet take weapon features into account:
    • shove action only appears for "hook" weapons
    • parry has a penalty for non-parrying weapons (it appears both in the action label and the roll dialog)
  • Migration of the old data: features -> features.others.

And that's about it. Tell me what you think about this! :)
Addresses #19

Change the weapon features from a single string to an object containing
all features available in the core rules.
 * melee weapon features: edged, pointed, blunt, parrying and hook
 * ranged weapon feature: slow reload

A free-form list is left to allow custom features to be added to
weapons.

The weapon sheet was modified to let users select the weapon features
via toggles. Available toggles depend on the current weapon's category
(melee or ranged).

There are also some changes in the way weapons are posted to the chat,
mostly to allow the localization of stock weapon features.
As stated by combat rules: "If you PARRY with a weapon that lacks the
PARRYING feature, you get a -2 penalty."

The parry action link in the combat tab will display the penalty if a
melee weapon lacks the appropriate feature.
Weapon features are now objects instead of a string. The migration was
kept simple: the previous value is moved over to the "others" field
(which as free-form string).
@giant-teapot giant-teapot added the feature New feature or request label Feb 28, 2021
@giant-teapot giant-teapot added this to To be Reviewed in Forbidden Lands Development via automation Feb 28, 2021
@aMediocreDad
Copy link
Member

Great! I'm looking forward to checking it out:)

Will hopefully get to it tomorrow.

Copy link
Member

@aMediocreDad aMediocreDad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great QoL change to the system here! ⭐🌟

Have left some thoughts on certain implementations. But overall happy with how it looks 😀

Will come back with suggestion for visual layout/styling.

script/hooks/migration.js Outdated Show resolved Hide resolved
script/hooks/migration.js Outdated Show resolved Hide resolved
style/item.css Show resolved Hide resolved
script/sheet/item.js Show resolved Hide resolved
@aMediocreDad aMediocreDad moved this from To be Reviewed to Being reviewed in Forbidden Lands Development Mar 3, 2021
@aMediocreDad aMediocreDad self-assigned this Mar 3, 2021
@aMediocreDad aMediocreDad added this to the 4.3.0 milestone Mar 3, 2021
@aMediocreDad aMediocreDad mentioned this pull request Mar 3, 2021
13 tasks
@aMediocreDad aMediocreDad linked an issue Mar 5, 2021 that may be closed by this pull request
13 tasks
- The migration changes are compatible with the official content and likely other English worlds.
- WorldSchema is now an integer and failsafes are built in for schemaVersions larger than system version.
@aMediocreDad aMediocreDad self-requested a review March 10, 2021 20:19
Copy link
Member

@aMediocreDad aMediocreDad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have taken care of the changes I requested, and have reviewed the changes using the official modules, as well as I can. Will go ahead and merge and close this one.

Great addition to the system! Thanks @giant-teapot

@aMediocreDad aMediocreDad removed the request for review from jimorie March 10, 2021 20:29
@aMediocreDad aMediocreDad merged commit c9fb37b into fvtt-fria-ligan:main Mar 10, 2021
Forbidden Lands Development automation moved this from Being reviewed to Next Release Mar 10, 2021
@giant-teapot
Copy link
Collaborator Author

Ah, I finally got a moment to myself and as I was about to work on those final touches and I just saw you already took care of it!
Thanks again @aMediocreDad, you rock! 😄 🎸

giant-teapot added a commit to giant-teapot/forbidden-lands-foundry-vtt that referenced this pull request Mar 13, 2021
Fixes a small UI bug introduced with fvtt-fria-ligan#47: only weapon's additional
("others") features would appear in the character sheet's combat tab.

This fix reverts back to the old behaviour as all weapon features should
appear for each eligible items.
@aMediocreDad aMediocreDad moved this from Next Release to Archive in Forbidden Lands Development Mar 20, 2021
aMediocreDad added a commit to aMediocreDad/forbidden-lands-foundry-vtt that referenced this pull request Apr 7, 2021
aMediocreDad pushed a commit to aMediocreDad/forbidden-lands-foundry-vtt that referenced this pull request Apr 7, 2021
Fixes a small UI bug introduced with fvtt-fria-ligan#47: only weapon's additional
("others") features would appear in the character sheet's combat tab.

This fix reverts back to the old behaviour as all weapon features should
appear for each eligible items.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5.0.0 Release Weapon actions in the character sheet should take into account weapon features
2 participants