Remove trailing spaces from all fillers#8
Conversation
|
Sorry I missed this PR (I did not receive mail notification), do you think we should merge? |
|
Not quite yet, the PR message does discuss more options that might be better than what's presented in the PR (but of which I don't know how to implement yet). |
|
I also realise the might be an edge case of having no instructions after 0xFF instruction within same box name but the code still continues. e.g. |
|
I am not sure to understand the edge case? So if I summarize we would like a config like: But currently it does not quite work because:
Am I right or did I miss something? |
|
Let me know if the below still isn't clear.
Basically there is advice going around that tells people to partially disregard the output of the CodeGenerator, e.g. removing trailing spaces or replacing empty box names within codes. While this advice works most of the time, there are a few codes that will not work as expected precisely because users didn't type them out exactly as intended. By having CodeGenerator remove the trailing spaces and shorten box names for the user, the user can just type the box names as is rather than risking a game crash by not typing the box names as is. Because shortening these box names also come with benefits such as typing the box name faster. It also lessens mistakes from users who didn't shorten their box names properly that leads to a game crash.
Kind of, though I fear that particular configuration may lead to this error: The code for above example would look something like |
|
I improved the way This makes it possible to use these fillers: which I think do what you want except that empty box names 2, 6, 10, 14 will be For instance the code you mentionned: with the exit code generates what you said, except for the difference I mentionned. Would this be okay or do you really need all empty boxes to be only Also I haven't tested it extensively so please tell me if you find an issue with this filler configuration. |
|
Single Edit: I noticed a bug where |
|
I made the box fitting algorithm more flexible, now your example works: |
|
What kind of work would be needed to allow something like a single |
|
I do not think it can be handled by the "box fitting" algorithm, because when it inserts a filler it does not know yet whether there will be instructions after in the box name or not. So maybe the easiest would be to use the fillers I mentionned above, together with some additional box name rewriting rules (e.g. |
|
@it-is-final I implemented some rewritting rules. It can be changed in the headers but I have added some default ones to implement the behavior you want. Can you check if codes with the following headers are written as expected? |
|
As far as I can tell, the new fillers seem to work pretty well. I tried it with codes (e.g. Make any Pokemon shiny) that became bugged (from reading the Discord) the last time you changed the code that dealt with fillers, and they work. I think I can now force push this branch to have those filler settings be default. |
2 participants
This pull request implements removing all trailing spaces from the default fillers in Boxes 2, 3, 4, 6, 7, 8, 10, 11, 12, and 14 across all game versions, using the more conservative option. This should allow typing codes to be slightly easier and faster as the user will no longer need to double check their trailing spaces, only the leading ones. Though something will need to be done about possibly prettifying the box name codes output from the "Result" when trailing spaces are removed.
At the moment, the common prevailing practice for the Switch rereleases of FireRed and LeafGreen is removing all trailing spaces and replacing
_ _ _ … _ _ _ _fillers withA. However this new advice (meant specifically for Switch versions) is inconsistent with the older practice of writing all box name codes as exactly shown in the CodeGenerator.While this advice of removing trailing spaces works fine for the vast majority of codes, blindly following this advice (and only for Switch version) has the potential to break codes that depend on these trailing spaces, with one example brought up being some of Sleipnir17's codes. Furthermore the advice of removing all trailing spaces (with exception to the ones in Box 4, 8, and 12 which were already removed for FR/LG in the CodeGenerator) does not seem to have a technical basis that makes that makes this practice only applicable to the Switch (which is how it is presented right now), because removing them actually works fine on all games and platforms for most codes.
By making removing trailing spaces the default, they will only be shown when they are needed for the code to function as intended, making clear in the code when they are required. This means that people can continue following the advice of writing box names exactly as shown, while benefiting from the optimisation that comes with removing trailing spaces. Users might need to be retrained to actually type the box names exactly as shown so that the required trailing spaces are typed in, rather than manually removing trailing spaces.
Codes that might break from this change really should have defined an int32 within the code, rather than relying on the filler's default value for their functionality.
There has been further discussion in the Glitch City Research Institute server to potentially replace these fillers with the ones shown below. These have the benefit of being easier to type and debug than spaces and
…. This means less chance of a crashing typo from missing a space or misplacing a…. However the singularAfiller might be a bit of work to implement in CodeGenerator, but would be easier to type than a variant of_ _ _ … _ _ _ _. These fillers would fail in the exact same conditions as the current fillers for Ruby and Sapphire since these areLTcondition code fillers, as opposed to theEQcondition code (used in FR/LG and Emerald).A(no spaces after)* * * *(no spaces after)A A A * * * *(no spaces after)A A * * * *(no spaces after)A * * * *(no spaces after)