Skip to content

Ensure control points using "recovery" and "decay" work as intended#915

Merged
Electroid merged 1 commit into
PGMDev:devfrom
calcastor:decay-recovery-parse
Aug 25, 2021
Merged

Ensure control points using "recovery" and "decay" work as intended#915
Electroid merged 1 commit into
PGMDev:devfrom
calcastor:decay-recovery-parse

Conversation

@calcastor

@calcastor calcastor commented Aug 24, 2021

Copy link
Copy Markdown
Contributor

#864 reintroduced decay and recovery rates to control points from 1.9+ PGM, while also introducing the possibility for already captured control points to decay from a captured state. However, ControlPointParser only recognizes and parses recovery-rate and decay-rate, despite 1.9+ PGM's implementation using recovery and decay. This PR ensures compatibility with maps previously made for 1.9+ PGM by making sure recovery and decay are also recognized.

I've also made sure that owned-decay can also be used alongside owned-decay-rate, to keep wording consistent between all 3 attributes.

@calcastor calcastor requested a review from Electroid as a code owner August 24, 2021 08:50
Signed-off-by: BT (calcastor/mame) <43831917+calcastor@users.noreply.github.com>
@calcastor calcastor force-pushed the decay-recovery-parse branch from e165d22 to 2262e57 Compare August 24, 2021 08:52

@Pablete1234 Pablete1234 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice catch, didn't notice we weren't complying with the 1.9+ pgm standard when we used these names.

For reference, the original names were definetly recovery and decay:
https://github.com/OvercastNetwork/ProjectAres/blob/master/PGM/src/main/java/tc/oc/pgm/controlpoint/ControlPointParser.java#L77

@Electroid Electroid merged commit 4be0024 into PGMDev:dev Aug 25, 2021
@calcastor calcastor deleted the decay-recovery-parse branch August 28, 2021 22:11
@CoWinkKeyDinkInc

Copy link
Copy Markdown
Contributor

It seems this PR has broken the dropper maps which use recovery="false".

@calcastor

Copy link
Copy Markdown
Contributor Author

It seems this PR has broken the dropper maps which use recovery="false".

Correct me if I'm wrong, but while the 1.9+ PGM docs state that true and false were acceptable values for recovery and decay, the code in 1.9+ PGM seen here doesn't seem to imply that those ever functioned as acceptable values. In other words, I don't think these maps were using the attribute correctly to begin with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants