tmedwards / sugarcube-2

SugarCube is a free (gratis and libre) story format for Twine/Twee.
https://www.motoslave.net/sugarcube/2/
BSD 2-Clause "Simplified" License
185 stars 42 forks source link

Config.cleanupWikifierOutput is sometimes ignored #189

Open gulumige opened 2 years ago

gulumige commented 2 years ago

I had Config.cleanupWikifierOutput set to true on one of my projects and I noticed that whenever I used Engine.show() it wouldn't convert the <br> elements to <p> elements. It also didn't seem to have any effect on the body of the Dialog except for adding a lot of extra <p> elements to the Settings menu which was a bit annoying. It did seem to work as expected when calling SugarCube.Engine.show() from the console in any browser I tried. (same with SugarCube.Dialog...).

I decided to take a deeper look and, while it's a bit over my head, from what I gathered is that if any Engine navigation methods or the Dialog.wiki() method is executed from "wikifying" the content inside of a macro like <<link>> or <<button>> it will not "clean up" the incoming text. I did also discover the convertBreaks function and figured it could be as both a bandaid fix and to demonstrate the issue.

JS
Config.cleanupWikifierOutput = true

:: Passage
<<link "Unclean">>
  <<run Engine.show()>>
<</link>>

<<link "Clean">>
  <<run convertBreaks(Engine.show())>>
<</link>>

:: StoryMenu
/* the common way of showing a dialog */
<<link "Unclean">>
  <<script>>
    Dialog.setup("Unclean");
    Dialog.wiki(Story.get("Passage").processText());
    Dialog.open();
  <</script>>
<</link>>

/* this produces the expected result imho */
<<link "Clean">>
  <<script>>
    Dialog.setup("Clean");
    Dialog.wiki(Story.get("Passage").processText());
    convertBreaks(Dialog.body());
    Dialog.open();
  <</script>>
<</link>>

Additional context. Not sure if this is really a bug or more of a side effect but I figured I'd report it.

tmedwards commented 2 years ago

Config.cleanupWikifierOutput is a temperamental beast.

Due to various reasons it is only called for top-level parser (Wikifier) calls—meaning those without a Wikifier ancestor. That's what's biting you in most (all?) of the cases you encountered.

It also doesn't handle out-of-band content injection terribly well.

Anyway. There are, unfortunately, no good or simple fixes for its issues with the current recursive/chunky parser.