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

fix: Handle "relative" and "proportional" with encumbrance #5858

Merged
merged 2 commits into from
Jan 6, 2025

Conversation

Coolthulhu
Copy link
Member

@Coolthulhu Coolthulhu commented Jan 1, 2025

Checklist

Required

Optional

Purpose of change

Fix #3861

Describe the solution

Use assign in a hacky way to have it handle the fields properly.

  • Add tests that confirm it works

Describe alternatives you've considered

Revert the armor portions thing - it won't help us much, but it is certain to cause trouble until we rewrite the jsons to the new format.

Testing

  • Spawn chitin gauntlets and biosilicified chitin gauntlets
  • Compare encumbrance - biosilicified should have 15 encumbrance, while regular should have 10
  • Load world with Magiclysm
  • Spawn chitin gauntlets and demon chitin gauntlets
  • Demon chitin gauntlets should have 9 encumbrance, while regular should have 10

Additional context

@github-actions github-actions bot added the src changes related to source code. label Jan 1, 2025
@github-actions github-actions bot added JSON related to game datas in JSON format. mods PR changes related to mods. labels Jan 6, 2025
@Coolthulhu Coolthulhu marked this pull request as ready for review January 6, 2025 14:01
@scarf005 scarf005 requested a review from chaosvolt January 6, 2025 14:41
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

LGTM, but playtest needed @chaosvolt

@chaosvolt
Copy link
Member

  • Compare encumbrance - biosilicified should have 15 encumbrance, while regular should have 10

Invalid for testing since I manually set encumbrance overrides for stuff that was broken by this bug in vanilla:

  {
    "id": "gauntlets_acidchitin",
    "copy-from": "gauntlets_chitin",
    "looks_like": "gauntlets_chitin",
    "type": "ARMOR",
    "name": { "str": "pair of biosilicified chitin gauntlets", "str_pl": "pairs of biosilicified chitin gauntlets" },
    "description": "Gauntlets crafted from the carefully cleaned and pruned biosilicified exoskeletons of acidic ants.  Acid-resistant and very durable.",
    "material": [ "acidchitin", "leather" ],
    "price_postapoc": "1750 cent",
    "encumbrance": 15,
    "proportional": { "weight": 1.125, "volume": 1.13, "price": 1.25, "warmth": 1.5 },
    "relative": { "bashing": 1, "material_thickness": 1, "environmental_protection": 1 }
  },

Demon chitin items do still have encumbrance modifiers since I forgot about magiclysm though.

Copy link
Member

@chaosvolt chaosvolt left a comment

Choose a reason for hiding this comment

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

  1. Compiled and load-tested.
  2. Loaded up a save with magiclysm.
  3. Compared chitin armor with demon chitin armor, they correctly show different encumbrance now.

As noted, we'll want to reimplement this in vanilla for items we had to work around the broken feature with in the future.

@chaosvolt chaosvolt merged commit dfe8ee3 into cataclysmbnteam:main Jan 6, 2025
13 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON related to game datas in JSON format. mods PR changes related to mods. src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

proportional and relative no longer update encumbrance at all
3 participants