Nameless temporary lables should be banned from s2disasm

[Awuwunya]

I am going to make the claim that nameless temporary labels are harmful for every single usage this disassembly may have. These are the +, - and / labels, that are littered around the disassembly, and make reading and maintaining code a horrible experience. Why?

1. They do not obey scope rules quite the same as other labels. Temporary labels are limited within label scope (but may be specially addressed from outside of the scope if needed), while normal labels create a scope between each other, and macros and equates work roughly the same as normal labels as well. Nameless temporary labels do not respect label scope, but also do not create their own scope. This behavior is unusual at the very least.
2. They promote unmaintainable code. Just even go look at KosDec inside s2.asm. Tell me if you'd enjoy modifying this code in case you needed to add/remove something? You need to read the entire routine just to make sure that the nameless temporary labels match after your alterations - or - not add any new temporary nameless labels or remove existing ones just to be sure it will keep working. Especially for long methods that rely on skipping over these labels, as is the case with KosDec and even worse abominations elsewhere, it becomes very difficult to tell when you have missed something or made a mistake
3. You lose the assembler's built-in detection for missing or duplicate labels. Although normal temporary labels might not exactly be easy to name or come up with any helpful names, at least if you delete a label another piece of code is using, you will get an assembler error. Similarly, these help catch many other similar issues relating to modifying code, so you are aware of when you actually have made a mistake. This is impossible with nameless temporary labels.
4. You are promoting poorly documented code to newcomers and possibly even intermediary programmers. Having disassemblies show an example of code that is of poor quality intentionally is not a good way to promote people to pick up good habits such as documentation or making code that is actually maintainable. Although this isn't what disassemblies in their current state can be a gold standard on, can we at least pick out some of the most obvious bad habits we are likely to be teaching newcomers? Temporary labels have the possibility to at least give some hints about how the code flow was intended to function, so its better than just seeing a single symbol appended to a branch instruction.
5. The purpose of temporary nameless labels are not at all self-explanatory. What does + mean? What does - mean? How does / both work as - and + in a visually confusing manner? Yeah. This is not only a confusing and unconventional manner of defining labels, it straight up can't even follow its own rules in a way that would make it easy to use. / screws up any train of logic you might have, promoting you to make even more mistakes. Again, KosDec.
6. Nameless temporary labels do not respect label scope. This means a nameless temporary label $1000 bytes and 20 labels later is just as valid of a target as one few instructions away. There is no obvious flow to how they work or what their restrictions are. The system with temporary labels and normal labels works because temporary labels are always within a reaching distance, and normal labels are expected to have a range from few bytes to literally anywhere in the ROM. nameless temporary labels have the expectation of having the same limitations as temporary labels, but actually the assembler doesn't care.
7. Nameless temporary labels will make effective debugging a pain too because you may be unable to understand how you ended up somewhere, and even if you look upon the code which caused an issue, unless you think about it enough, you may discount the possibility that the amount of +'s doesn't actually match with the routine. These also provide complications with debugging tools, such as listings files and in-ROM debuggers, as these labels are both abundant and completely unhelpful.
8. Anyone either reading or writing code will find it more difficult to understand code flow that contains nameless temporary labels, because they can't just scan for the appropriate label, but they have to count the amount of symbols in most cases. For some conservative situations, for example jumping past a few lines its fine, but for routines that contain multiple +'s, it adds visual noise and requires more attention on the code flow. This does not actually help however, simply because a lot of the time you aren't reading into routines to understand their specific operation, but skimming over code to understand what it does roughly, or if the code is relevant to the programming question you are researching. It only slows you down and may take your attention away from the problem you are having to counting the +'s.
9. The only reason to use it is laziness. I said it. You use it because its faster to write and requires less thinking. That attitude for sure isn't going to lead better code be written, or higher quality research to be achieved. The fundamental problems with these nameless temporary labels are to be ignored because its easier? That might be true for individual programmers, and more power to them, but as far as the official disassemblies are concerned? That is a ridiculous ideal to hold..

I knew I went on lengths about my severe gripes about the nameless temporaries and my problem with them, but having been tasked to maintain code littered with these devils has truly made me hate their use. Although we can't stop people from actually using it, packaging it in as the de-facto style to do temporary labels within disassemblies is absolutely unreasonable and I really think they should be outright banned from s2disasm and any other Sonic Retro disassemblies. I would even be happier with nonsense like .a, .b, .1, .derp, .asdasdsaasd, because it still retains many of the real world usage benefits mentioned above. Nameless temporary labels fail at every level to make any sense for anything but laziness of the user, and I don't think that should be the direction the disassemblies should take for sheer convenience for the submitter. Please discuss below

[lilggamegenius]

The only time I can see a nameless temporary label being useful at all, is for only for + and only when its used to go straight to a rts and even then you can just use a simple .ret or whatever

[Brainulator9]

Another problem is that if for some reason you want to port code over to ASM68K, it'll be more troublesome than just replacing periods with at signs.

For my purposes, I'd rather give everything a name, especially when the point of these projects is to document pre-existing code. If someone has trouble coming up with a name, someone else can take up the task.

[flamewing]

Agreed. I think that on a first pass, getting rid of all ---, --, ++, and +++ labels would already be a large improvement. For reference:

• s2.asm:
  • 304 instances of ++
  • 45 instances of +++
  • 35 instances of --
  • 4 instances of ---
  • 18 instances of / (can be reached by both + or -)
• s2.sounddriver.asm:
  • 1 instance of ++ as a label (appears in comments)
  • 0 instances of +++
  • 0 instances of -- as a label (appears in comments)
  • 0 instances of ---
  • 0 instances of / (can be reached by both + or -)

[Awuwunya]

Another problem is that if for some reason you want to port code over to ASM68K, it'll be more troublesome than just replacing periods with at signs.

And in actuality, ASM68K already supports dots as the local label symbol (which, IMO, should be what we're using for disassemblies anyway, but that's a whole other topic)

Also I agree with what Flamewing says

[Brainulator9]

As a proof-of-concept of sorts, I took the liberty of removing every nameless temporary label from the sound driver, which you can see here. I'm thinking of just putting this in a pull request alongside the rest of the game, but I wonder if this sort of thing would be a pain for you all to look over.

[flamewing]

Making a separate pull request for the sound driver would already be a huge help, especially with regards to reviewing it..

[Brainulator9]

I guess I'll do something like that... I got ahead of myself.