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

Fixed 'null' in WordScaleQuality lists not behaving as you'd expect #40

Merged
merged 1 commit into from Mar 1, 2015

Conversation

sequitur
Copy link
Contributor

@sequitur sequitur commented Mar 1, 2015

One of the features of QualityDefinition is that if the format() method returns null, nothing at all is written and the quality isn't displayed at all. You would expect that if one of the items in a WordScaleQuality is null, then that quality shouldn't be displayed at all as long as the quality is at that point in the scale; this is useful, for instance, for a quality that should only display if it is greater than some quantity (Eg, tracking wounds the player character suffered), a quality that depletes until it disappears entirely (Eg, tracking how much of a finite resource the player character has left), or a quality that is only visible if it is exceptional (Eg, in a game with many stats, showing only stats that aren't average).

Unfortunately, Undum currently doesn't behave this way. The reason why is line 479 in undum.js:

  return this.values[val] + mod;

Because mod is always a string, this line type coerces this.values[val] into a string, so that null becomes 'null'. I've added a line to check for null here and return an actual null value if that's the case, thus making it so the expected behaviour actually happens, making the interface here more consistent with the other QualityDefinition variants.

@idmillington
Copy link
Owner

Perfect. Thanks for the spot, and thanks for the fix!

idmillington added a commit that referenced this pull request Mar 1, 2015
Fixed 'null' in WordScaleQuality lists not behaving as you'd expect
@idmillington idmillington merged commit 56c080c into idmillington:master Mar 1, 2015
@sequitur sequitur deleted the quality-list-null branch March 1, 2015 15:54
@sequitur
Copy link
Contributor Author

sequitur commented Mar 1, 2015

Great, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants