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 footnotes in Flee Mortals #1869

Closed
wants to merge 1 commit into from

Conversation

ebullient
Copy link
Contributor

Use the footnoteEntries attribute in spellcasting to explain symbols for casting time

Use the footnoteEntries attribute in spellcasting to explain symbols for casting time
@TheGiddyLimit
Copy link
Owner

TheGiddyLimit commented Oct 15, 2023

"Fix" is a strong word; what, in particular, is wrong with the current version? The @footnote usage was a specific choice I made, and even added to the creature converter!

@TheGiddyLimit TheGiddyLimit self-requested a review October 15, 2023 13:40
@ebullient
Copy link
Contributor Author

ebullient commented Oct 15, 2023

"Fix" is a strong word; what, in particular, is wrong with the current version? The @footnote usage was a specific choice I made, and even added to the creature converter!

I concede; "Fix" is a strong word. "Propose alternate way to do this" is better.

Most of the time, I can make beautiful markdown notes out of 5eTools JSON data. This particular structure causes problems.

  1. You're assigning the footnote value to a pre-formatted superscript element. If I need to use markdown footnotes, I could do that here (I'd have to strip the superscript formatting. I originally took that route), but if I do that..
  2. I have several footnotes with the same key (A, +) and the same value (typed several times).

I could make assumptions that any footnotes with the same key will have the same value and shove them into a map and render them before or after footerEntries, but that seemed pretty brittle.

So I thought I would suggest this approach instead. You still have the casting time indicators in the spell list, using superscript. And there is a single element in the footerEntries that explains those indicators. If you hate this plan, I'll go back to stripping the superscript formatting and will figure out some way to collapse footnotes with identical values and throw errors if the same footnote identifier is used with different values.

As of now, this is the only file in the Homebrew repo that uses this structure. So I figured now would be the time to propose an alternative. ;)

@revilowaldow
Copy link
Contributor

For context, ebullient makes the 5eTools (& PF2e) to Obsidian Markdown tool.
https://github.com/ebullient/ttrpg-convert-cli

@TheGiddyLimit
Copy link
Owner

For context

ty I did remember this time


If you hate this plan

"Hate" is another strong word, but I do like my nice hover-able superscript letters. That said, on closer inspection, the @footnote tag is a bit broken; it relies on some in-memory hackery and so doesn't work when e.g. importing a creature via Plutonium and then refreshing the page.

I think my path of least resistance here would be to make it into a loadable entity type i.e.

"footnoteEntry": [
  {
    "name": "Spell Action Note",
    "source": "FleeMortals",
    "page": 123,
    "entries": [ ... ]
  },
  ...
]

and then @footnote would work like any other entity-loading tag (@spell, @creature, etc.) and so we'd have something of the form:

"{@spell detect magic}{@footnote {@sup A}|Spell Action Note|FleeMortals}, {@spell disguise self}{@footnote {@sup A}|Spell Action Note|FleeMortals}"

This would fix the Plutonium jank (For Free:tm: by way of the existing hover-able tag handling paths), and de-dupe all the footnote strings. I imagine you could then decide how you wanted to deal with linked @footnotes in an object; you could scan all the strings and pull out the tags and render them in a bit at the end, or, ... .

Thoughts?

@ebullient
Copy link
Contributor Author

ebullient commented Oct 15, 2023

If you hate this plan

"Hate" is another strong word, but I do like my nice hover-able superscript letters.

=) Fair enough. The hover is definitely nice for the web UI, it just doesn't work well in Obsidian or straight markdown.

I think my path of least resistance here would be to make it into a loadable entity type i.e.

"footnoteEntry": [

    "name": "Spell Action Note",
    "source": "FleeMortals",
    "page": 123,
    "entries": [ ... ]
  },
  ...
]

and then @footnote would work like any other entity-loading tag (@spell, @creature, etc.) and so we'd have something of the form:

"{@spell detect magic}{@footnote {@sup A}|Spell Action Note|FleeMortals}, {@spell disguise self}{@footnote {@sup A}|Spell Action Note|FleeMortals}"

Does this break existing footnotes (I think you're saying it does, and you want to change all of the existing footnotes?):

  • {@footnote directly in text|This is primarily for homebrew purposes, as the official texts (so far) avoid using footnotes}
  • {@footnote optional reference information|This is the footnote. References are free text.|Footnote 1, page 20}

For example:

Rorreth (Red Wizard) has the following {@footnote wizard spells prepared|Replaced non-existent {@b fire blast} spell from original text with {@b fire bolt}}...

Can we do something like {@footnoteRef key|FootNote Name|source} instead, where that key is the known/fixed superscipt value (no {@sup..})?

(that would mean you could leave all other uses of {@footnote..} alone)

@revilowaldow
Copy link
Contributor

revilowaldow commented Oct 15, 2023

There's over 500 uses of {@footnote in the homebrew repo, as long as you have a plan to migrate them I don't have an objection.
Though I do think that here, where they're all the same, it's easy. But making a dedicated object for each individual footnote in a brew where they're used throughout the text sounds like pain incarnate.
image

See also {@homebrew tags if you're hoping to factor this similar tag behaviour out.
image

@ebullient
Copy link
Contributor Author

ebullient commented Oct 16, 2023

Given the changes currently in the PR, I would generate something like this (appearance varies greatly by theme):

image

I agree that the lack of hover over the little A and the + is sad, but there is a not a great way for me to add that using normal footnote processing (as you realized, I think).

I can make a footnoteRef work. I think there is a similar situation in tables (well-known/reused footnote key and a definition of what that key means in the table footer). So maybe there is another noun that would be better to indicate superscript/linked references to localized/nearby footer content (as another thought)

@TheGiddyLimit
Copy link
Owner

TheGiddyLimit commented Oct 16, 2023

Does this break existing footnotes

a plan to migrate

yes and yes, they're trivial enough to migrate

Can we do something like {@footnoteRef key|FootNote Name|source} instead

apologies; smashed out meaningless junk before going to bed. What I meant was: {@sup {@footnote Spell Action Note|FleeMortals|A}} which is essentially what you said but wrapped in a styling tag (currently valid syntax elsewhere, e.g. {@i {@spell ...}})

having both @footnote and @footnoteRef

that's a definite hate. I'm not maintaining two systems for the same thing, especially for something as uninteresting as footnotes

pain incarnate

is it really, though? Make one/or an IDE template of your choice with a generic name (say "name": "Footnote p#"\n"source": "..."), and it's about as quick as the current system, no..? Presumably one has chop up text currently to inline a footnote so I don't see this as a huge workflow change

@homebrew tags

those are likely broken too, but iirc the implementations are separate, so we can table that


really what I'm asking is, "is this any different to a @skill/@sense tag." From an implementation standpoint, it's a firm "no," and I'm hesitant (aka it ain't happening) on making things more complex by adding e.g. block-local references. If @footnoteRef is workable then that can be the new @footnote!

@revilowaldow
Copy link
Contributor

it's about as quick as the current system, no..?

I agree, it's fine in terms of the actual technical challenge of making an entry.

The pain is that in a brew with 80 unique footnotes I now need to come up with 80 source-unique "name"s for what is effectively an anecdotal sentence.
When you migrate these, are you going to come up with sensible names or automate them 1 through 500? Using the nearest entry header might help but multiple footnotes within an entry are non-unique and the entry header could well be completely disconnected from the anecdote contained in the footnote.

I thought the point of footerEntries on a Spellcasting entry was that this is how wizards and inspired brew sources laid out the notes for spell lists?

Changing the whole footnote system which is used very differently in other parts of the site to enable this specific brew feels strange. Though I understand the wish to factor out non-standard behaviour.

@TheGiddyLimit
Copy link
Owner

I expect a "footnote" to be something like this:

image

I now need to come up with 80 source-unique names

so the "uniquely named entity version" of it would just be this:

"footnoteEntry": [
  {
    "name": "Footnote 17",
    "source": "MySource",
    "page": 250,
    "entries": [ ... ]
  },
  {
    "name": "Footnote 18",
    "source": "MySource",
    "page": 250,
    "entries": [ ... ]
  },
  ...
]

or for a book which doesn't uniquely number their footnotes:

"footnoteEntry": [
  {
    "name": "Footnote 1 p250",
    "source": "MySource",
    "page": 250,
    "entries": [ ... ]
  },
  {
    "name": "Footnote 2 p250",
    "source": "MySource",
    "page": 250,
    "entries": [ ... ]
  },
  ...
]

footerEntries on a Spellcasting

agreed..? Not sure I follow, this is not about changing footerEntries (no plans there)

Changing the whole footnote system which is used very differently in other parts of the site to enable this specific brew feels strange. Though I understand the wish to factor out non-standard behaviour.

I'll contest this:

  • the experience, as a user, of browsing the page is ~identical under either implementation. For the vast majority of people it is therefore not a "change to the whole system." As an implementation, it is a refactor
  • the specific brew is incidental. This move, most importantly, fixes Plutonium (in which footnotes are 100% broken); and (hopefully) alleviates a third-party issue. It does both of these while also simplifying the implementation (at least on the 5etools/Plutonium side) to be inline with various other existing things, which is a win
  • if footnotes are being used as something other than the intended "page footnotes" pic'd above, then that is a failure of the brew, not a failure of the system..? There are other tools for similar uses (@note, @homebrew), and/or, as per above, this is "refactor" not "rework" so everything should go on functionally working as it always has

@TheGiddyLimit
Copy link
Owner

as an aside I think @footnote was literally added for Strongholds & Followers, so rejigging it to suit Flee Mortals seems apt 😼

@ebullient
Copy link
Contributor Author

ebullient commented Oct 16, 2023

apologies; smashed out meaningless junk before going to bed. What I meant was: {@sup {@footnote Spell Action Note|FleeMortals|A}} which is essentially what you said but wrapped in a styling tag (currently valid syntax elsewhere, e.g. {@i {@spell ...}})

Frankly, I think this is worse, and it really messes up what are otherwise functional footnotes everywhere else. This is the only source that does things this way, and I feel like you grabbed the footnote as the closest thing to what you wanted (not trying to put you on the defensive here, it did give you the nice hover behavior, so I can understand the choice).

as an aside I think @footnote was literally added for Strongholds & Followers, so rejigging it to suit Flee Mortals seems apt 😼

The {@footnote..} tag has been in the schema as text for a very long time, and is in much wider use. It works with what most Markdown systems can already do (generating footnote numbers so folks don't have to maintain a count).

This particular usage (in FleeMortals) is inconsistent with the others. Technically, it's a cross-reference to a notation convention in the source material. You could do something like {@notation .. }, which would be more appropriate for this use, and leave all of the other footnotes alone.

that's a definite hate. I'm not maintaining two systems for the same thing, especially for something as uninteresting as footnotes.

I don't think this is the same thing. Maybe {@notation .. } is less hate-worthy.

I thought the point of footerEntries on a Spellcasting entry was that this is how wizards and inspired brew sources laid out the notes for spell lists?

It seemed footerEntries was the right place for this, which is why I suggested adding it there.

Changing the whole footnote system which is used very differently in other parts of the site to enable this specific brew feels strange.

It really does. Most of those other footnotes are very contextual (none of them match each other). I agree that changing all footnotes to deal with this case (where this case is the outlier) seems strange.

@TheGiddyLimit
Copy link
Owner

Frankly, I think this is worse

Technically, it's a cross-reference to a notation convention in the source material.

fair enough, and yes, that was what I was thinking of (e.g. wiki-like [2][3] references). If smushing those two ideas together is terrible then it could certainly be its own new thing (your @notation idea).

Changing the whole footnote system

I'll admit I'm not the one converting the brews; my cherry-picked example above just happens to be one of the PDFs I have sitting on my drive. In my head this sounded like a good idea, clearly, but apparently not

@revilowaldow got any handy you could screenshot/post snippets from? E.g. I see "Exquisite Exandria" has a lot of footnotes; what's the source text look like for those?

@revilowaldow
Copy link
Contributor

image

image

@ebullient
Copy link
Contributor Author

ebullient commented Oct 16, 2023

That example is actually a little tough, as it also defines what it thinks the footnote should be. ;)

Here is a more typical one:

image

Although (in this case), the text is the same (this is also in spells, so I feel like this could be another candidate for {@notation...}):

image

The source: https://raw.githubusercontent.com/TheGiddyLimit/homebrew/master/creature/Kobold%20Press%3B%20Creature%20Codex.json

@ebullient
Copy link
Contributor Author

From this one: https://raw.githubusercontent.com/TheGiddyLimit/homebrew/master/adventure/JVC%20Parry%3B%20Call%20from%20the%20Deep.json

We have text like this:

"Within the settlements of the Sword Coast one can find humans, dwarves, elves, and other civilised races commingling more or less peacefully. Waterdeep, in particular, is a melting pot of races from all over Faerûn. Outside of these communities, however, people have much less tolerance for other races, as folk tend to feel safer among their own kind. Small towns and villages dominated by humans tend to have few, if any, non-humans, with most dwarves, elves, and halflings preferring to live in their own settlements, far from human-claimed lands. In generations past, most human settlers of the Island Kingdoms were fair-haired and light-skinned. Since then, the riches and promise of the islands have attracted folk from other regions, and several generations of cultural mingling has given the humans of Faerûn much greater diversity in their appearance (see "{@footnote {@race human|phb|Human Names and Ethnicities}|See the {@i Info} tab of the {@race human} race.}" in chapter 2 of the {@i Player's Handbook})."

Which would appear as above. The footnote number would be generated (relative to the note), with footnotes at the bottom in the same way as shown above.

The same is also true for footnotes in collection/Kobold Press; Deep Magic 17 Mythos Magic.json, collection/Kobold Press; Tome of Heroes.json, as well as collection/MCDM Productions; Strongholds and Followers.json (which you noted earlier).

@TheGiddyLimit
Copy link
Owner

ty both, good stuff

So for the recipes, @footnote use more aligned with the footerEntries angle? I suppose in an ideal world more things would allow footerEntries and @footnote could go back to its original "in-the-footer page footnotes" role. Still, that'd be lots of effort for probably-negative gain, so if @footnote is the general tool for dealing with * ...'s then all good

As for the markdown, all looks reasonable to me. I imagine explicitly differentiating @notation and @footnote could help you format things nicely? That said, as this is a third-party use of """internal""" data this is not directly my concern...

I think the question then is, as I've been shouted back from the cliff-edge on the @footnote rework, do we like the idea of a @notation + "notation": [...] entity type? It's "free" enough to add/implement on my side, so I've no technical reason against doing so. It has a well-enough-defined use-case/precedence that it makes sense in the bigger picture?

@ebullient
Copy link
Contributor Author

As for the markdown, all looks reasonable to me. I imagine explicitly differentiating @notation and @footnote could help you format things nicely?

Yes!

That said, as this is a third-party use of """internal""" data this is not directly my concern...

😝

I think the question then is, as I've been shouted back from the cliff-edge on the @footnote rework, do we like the idea of a @notation + "notation": [...] entity type? It's "free" enough to add/implement on my side, so I've no technical reason against doing so. It has a well-enough-defined use-case/precedence that it makes sense in the bigger picture?

I think the @notation type would match to "notation": [...] very well, yes. Would allow reusable conventions for the whole source. Solves your Flee Mortals thing nicely, and means the meaning of A, and + is only defined once, and then reused/referenced throughout.

@revilowaldow
Copy link
Contributor

Sounds good to me, apologies if I've been shouting, 2°C Monday morning innit.

I won't be satisfied until get a
"refstyle": [ Harvard, APA, IEEE, MLA, MHRA ]

@TheGiddyLimit
Copy link
Owner

coolcool, will see to this after the new book's in and done, so will leave this PR until ~the weekend and close it when I've made the @notation (final name pending) version.

apologies

not at all; please do always shout. I cope by CSSing a funny moustache over your avatar.

"refstyle": [ Harvard, APA, IEEE, MLA, MHRA ]

😩

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.

3 participants