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.
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
Document Magic #1199
Document Magic #1199
Changes from 19 commits
7782db3
b5e6ac8
bf2858b
bdda1e5
6bd46b2
354c594
e359fce
80de240
522ee08
764a102
9125e97
e2eada8
f05f8c2
26adeef
bd3174d
0146061
aa10f47
fe133ce
9779f9e
727a5bd
5919acc
d647ba1
d992fb5
2ce2812
e0776f5
a920403
743bb67
ad2fbd9
ca12b3d
6a9468a
b9e8936
af5d5c9
34a7451
72306ba
c540364
c7225d1
a7a0d00
2bd92ab
b34a259
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realise this was used extensively in the repo, but now I'm wondering if we want to name variables like this interrogatively... does the
is
actually do anything here, that the plain past participle doesn't?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this specific instance,
doubleMagicAcquired
alone can be inferred to be a boolean-like value (it still uses the 0/1 in calculatingmagicCapacity
so it's not a true bool), but as you mentioned,is
is just a common and fast way to identify that something is boolean without thinking about it, at least that was my understanding of the conventionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also as a side note, not related to this comment but something I wanted to note:
doubleDefense
->isDoubleDefenseAcquired
(or without theis
) would be consistent withisDoubleMagicAcquired
but I refrained from changing it for nowThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we were going by the game it should be "half damage" (or possibly "enhanced defence") anyway:
(I suppose we could say the same thing for magic, actually, it never explicitly says it doubles, although that's obvious from the bar size)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo this is a case where the common colloquial name makes more sense. no one says half damage or enhanced defense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look into this, since I was curious. I looked at 4 guidebooks (3 English, 1 the official one, 1 Japanese) and the first page of Google results for walkthroughs, including 4 GameFAQs guides. A large majority say one or more of
and only 2 (of about 20) say double/doubled defense. I was pretty surprised by this myself, since the tiny speedrunning corner of the world does seem to call always call it "double defense", probably because of the alliteration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to add a different perspective, I come from the randomizer community and never heard of any of those terms, it's only ever been "double defense" for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other related names in decomp:
FAIRY_UPGRADE_HALF_DAMAGE
(hi elliptic),gGreatFairyDoubleDefenceCs
(hi louis)I would be okay with normalizing all of that to "half damage", with a comment on the save context struct member like
// double defense
"double defense" probably is so popular because it rolls so well off the tongue