feat(dr): opt-in cambrinth charge distribution across configured items - #1549
Open
simtel12 wants to merge 1 commit into
Open
feat(dr): opt-in cambrinth charge distribution across configured items#1549simtel12 wants to merge 1 commit into
simtel12 wants to merge 1 commit into
Conversation
DRCA charges cambrinth wrong when cambrinth_items holds more than one item.
This adds the fix behind a new per-character setting,
cambrinth_distribute_charges, and leaves the default path unchanged.
charge_cambrinth_items passes a flat charge list from YAML (cambrinth:
[4, 10]) to every item, so each item is charged the full amount. A 14 mana
plan charges 14 mana into the ring and 14 more into the armband.
calculate_mana, the use_auto_mana path, uses each item's cap only as a
ratio to pick a charge count, then gives every charge the same size. It
charges past the cap of the small items and drops the remainder of an
integer division, so mana goes nowhere. With caps 4/4/48 and 3 charges it
produces [[7], [7], [7, 7, 7]]: 7 mana into a 4 cap item, and 1 mana lost.
With cambrinth_distribute_charges set, allocate_cambrinth_charges gives
each item as much as its cap allows, in the order the profile lists them,
so a small worn item fills before a large stored one. Each item then splits
its own share into charges. Spare charges go to the largest charge while
that makes it smaller, which keeps each charge inside what the character
can channel at once. Mana that fits in no item returns to the base prep.
caps 4/4/48, 3 charges
13 mana to charge -> [[4], [4], [5]]
20 mana to charge -> [[4], [4], [12]]
36 mana to charge -> [[4], [4], [28]]
A flat charge list written in a profile is placed one charge at a time into
the first item with room, rather than skipping an item for good once a
charge has passed it by.
A single cambrinth item keeps the old behaviour even with the setting on.
In that path cambrinth_cap feeds the arcana check in
skilled_to_charge_while_worn? rather than a charge limit, and many profiles
charge well past it on purpose.
Cached discern data records the cambrinth caps it was calculated against,
so a change to cambrinth_items invalidates the cache instead of replaying a
distribution built for the old item list.
The setting is unset by default, so every existing profile keeps its
current behaviour. I checked that against all 270 profiles in dr-scripts
and my runtime scripts directory: with the setting off, both paths produce
byte-identical results to the current code for all 1638 spell entries that
carry a cambrinth list, and for calculate_mana across 55 distinct cambrinth
configs over 180 discern results each. With it on, 13 entries change, all
in the three profiles that configure several items.
calculate_mana no longer depends on the global NilClass#+ patch to build
its charge array. The behaviour is the same, and the default path is now
testable.
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
DRCAcharges cambrinth incorrectly whencambrinth_itemslists more than one item. There are two separate faults.charge_cambrinth_itemsrepeats a flat list on every item. A spell configured ascambrinth: [4, 10]reachescharge_cambrinth_itemsas a flatArrayofInteger. Thewhen Integerbranch passes the whole list to every item, so a 14 mana plan charges 14 into the first item and 14 more into the second.calculate_manaignores each item's cap. Theuse_auto_manapath usescaponly as a ratio to pick a charge count, then gives every charge of an item the same size. Nothing checks that an item's total fits its cap, and the remainder of an integer division is dropped. With caps 4/4/48 and 3 charges it produces[[7], [7], [7, 7, 7]]— 7 mana into a cap-4 item, and 1 mana lost.cast.licis unaffected, because it calculates the split itself before it callsDRCA.Change
A new per-character setting,
cambrinth_distribute_charges, selects a second code path. It is unset by default.With the setting on,
allocate_cambrinth_chargesgives each item as much as its cap allows, in the order the profile lists them, so a small worn item fills before a large stored one. Each item then splits its own share into charges. Spare charges go to the largest charge while that makes it smaller, which keeps each charge inside what the character can channel at once. Mana that fits in no item returns to the base prep.In the first and third rows the old code drops mana. In the third it also
asks a cap-4 item to hold 7.
A flat charge list written in a profile is placed one charge at a time into the first item with room, rather than skipping an item for good once a charge has passed it by.
Cached discern data records the cambrinth caps it was calculated against, so a change to
cambrinth_itemsinvalidates the cache. Without that, a distribution built for the old item list is replayed for up tocheck_discern_timer_in_hours, which charges one item and silently skips the rest.A single cambrinth item is left alone
Even with the setting on, one item takes its charge list unchanged. In that path
cambrinth_capfeeds the arcana check inskilled_to_charge_while_worn?rather than a charge limit, and many profiles charge well past it on purpose. Enforcing it there would quietly reduce charging for a large number of existing setups.Evidence that the default path is unchanged
I ran the patched module with the setting off against a verbatim copy of
main's algorithms and made any difference raise.charge_cambrinth_items: all 1638 spell entries carrying acambrinthlist, across the 270 profiles indr-scripts/profilesand my own runtime profile directory. No divergence.calculate_mana: 55 distinct cambrinth configurations over 180 discern results each. No divergence.With the setting on, 13 of those 1638 entries change. All 13 belong to the three profiles that configure several items, including
Samples/WarriorMage/Dartellum-setup.yaml, which today charges 240 mana for a 60 mana plan.One behaviour-preserving cleanup
calculate_manabuilt its charge array witharr[i] += 1on anilslot, which works only because Lich patchesNilClass#+. It is now written as(arr[i] || 0) + 1. Identical result, and the default path is testable without the global patch — which is why it had no coverage before.Testing
bundle exec rspec— 6383 examples, 0 failures. 60 new examples inspec/lib/dragonrealms/commons/common_arcana_spec.rb, five of which pin the default path so a later change cannot quietly flip the gate.bundle exec rubocop— clean.Companion change
elanthia-online/dr-scripts documents the new setting in
profiles/base.yamland teachescast.licto callDRCA.allocate_cambrinth_charges, so,castand the waggle scripts share one implementation. That PR checks the method exists and falls back to its own split on an older Lich, so it does not depend on this one landing first.Test plan
cambrinth_itemsand a flatcambrinth:list charges each item its own share instead of the full list.use_auto_manawith several items never charges an item past itscap.cambrinth_itemscauses the next cast to discern again rather than reuse the old distribution.