How do I make my code more "modular"?

Discussion of hardware and software development for Super NES and Super Famicom. See the SNESdev wiki for more information.

Moderator: Moderators

Forum rules
  • For making cartridges of your Super NES games, see Reproduction.
psycopathicteen
Posts: 3140
Joined: Wed May 19, 2010 6:12 pm

How do I make my code more "modular"?

Post by psycopathicteen »

https://forums.atariage.com/topic/35414 ... e/page/15/

Basically Turboxray claims my programming style is not "modular" enough, and he gives me a bunch of criticisms about my code.

How the heck am I supposed to predict in advance which parts of my code I would need to reuse a head of time? It seems like every time I try doing that my subroutines get more and more bloated because I would need to account for every single slight variation.
Pokun
Posts: 2675
Joined: Tue May 28, 2013 5:49 am
Location: Hokkaido, Japan

Re: How do I make my code more "modular"?

Post by Pokun »

I just took a quick look, but it looks to me that the criticism was part of pointing out things he needed for his argument in that discussion, regarding SNES only being able to handle 4 sprites at a time or something. I'm not sure what he meant by that, but I'm not really interested in getting into something that just looks like a fanboy discussion. His critics seems to mainly be about comparing your game to one back in the day isn't fair since you are not a developer from back in the day making a commercial game at the time with the same tool and time restrictions they had.

Turboxray tends to be blunt and is a big PC-Engine fan (which I am too BTW) but he also has good knowledge, so my advice is just to try to keep it cool and try to take it as constructive criticism even if it hurts. You already know what you have achieved so far, and there is always something good and something bad. You can't do everything perfect from the beginning but you can always become better at what you do no matter how good you are.


I'm not the right person to answer the question though. I'd guess by using modular parts like subroutines and avoid having too much dependency between them. Basically make the code be built up by (more or less) independent routines and subroutines.
Subroutines should probably also list input and output registers, a clobber list and list any other expectations they have so that they become easier to use in other projects or situations.


Someone in that thread also said we are unbelievably patient on the SNESdev forums, I take that as a compliment. :)
TrekkiesUnite118
Posts: 92
Joined: Fri Mar 08, 2013 5:56 pm

Re: How do I make my code more "modular"?

Post by TrekkiesUnite118 »

psycopathicteen wrote: Tue Nov 21, 2023 1:24 pm https://forums.atariage.com/topic/35414 ... e/page/15/

Basically Turboxray claims my programming style is not "modular" enough, and he gives me a bunch of criticisms about my code.

How the heck am I supposed to predict in advance which parts of my code I would need to reuse a head of time? It seems like every time I try doing that my subroutines get more and more bloated because I would need to account for every single slight variation.
Having not seen your code myself I can't really comment on it directly. But going off what turboxray posted I'm guessing there's a lot of hard coded magic numbers, no labels, single letter variable names, vague function names, misleading function names, etc. These are generally bad practices that lead to what people call spaghetti code that's pretty much single purpose, almost impossible to maintain, and can barely be read and understood by it's creator, let alone others.

Generally what I do when I'm writing and self reviewing my code is I think to myself "If someone who knew nothing about this code was dropped in front of it and tasked with learning and modifying/maintaining it, could they figure out what the code is doing based on what's written here?" If the answer to that is no, then you need to go back and start making it more readable and easier for others to figure out.

As for making it modular and predicting ahead of time, it's generally something that you just need to think about as you go. If there's something you're doing a lot, don't just copy paste it everywhere, make it a function or subroutine and call that instead. If you need to do some new thing, think about if you have something that already does something similar, and if you can leverage that and possibly modify it to be flexible enough to do both what it's already doing, and your new thing. Look at your code and break it down into what kind of job each bit of it is doing and start to organize it into common tasks/responsibilities. As you do that you may find that you have a lot of duplicate code, or very similar code that can be refactored and merged into something a little more flexible and reusble.

I think what turboxray was getting at was simply that back in the 90s, you may not have seen devs really pushing the system to the kinds of limits you're doing because they were generally on a tight schedule and in many cases were building off of previous projects and reusing code, tools, engines, etc. to just get something out the door on time that was good enough. Not having those constraints as a homebrew dev in your spare time you can get away with just brute force hard coding everything. But the end result there is code that's not really useable or understandable by others and possibly not even reusable by yourself if you find yourself wanting to make another game in the future. It also makes it more difficult if say you want to share your code with others to help them get started in SNES dev as it kind of boxes them into the exact way you've done something that may not actually suit their needs.

I know this sounds vague and generic but it really is just a kind of mindset. As you start thinking more about "Do I already have something that does this?" or "Could this be useful in the future if I make it a little more flexible?", etc. you just start to naturally think that way and design your code around it. So hopefully this makes it a bit easier to grasp what the criticism is. At the end of the day it's your code, and if it works for you that's fine.
tepples
Posts: 22705
Joined: Sun Sep 19, 2004 11:12 pm
Location: NE Indiana, USA (NTSC)
Contact:

Re: How do I make my code more "modular"?

Post by tepples »

Pokun wrote:Basically make the code be built up by (more or less) independent routines and subroutines.
See, for example, the doc comments for each subroutine in ppuclear.s in lorom-template.
creaothceann
Posts: 610
Joined: Mon Jan 23, 2006 7:47 am
Location: Germany
Contact:

Re: How do I make my code more "modular"?

Post by creaothceann »

You don't even have to use subroutines. As long as you have enough ROM space, macros can (almost) do the same job and are faster to execute.
My current setup:
Super Famicom ("2/1/3" SNS-CPU-GPM-02) → SCART → OSSC → StarTech USB3HDCAP → AmaRecTV 3.10
psycopathicteen
Posts: 3140
Joined: Wed May 19, 2010 6:12 pm

Re: How do I make my code more "modular"?

Post by psycopathicteen »

I'm trying to think of pieces of code that I've posted on here and I think Turboxray might've seen my dynamic animation routine and assumed most of my code looked like that. The only reason that particular routine was so complicated was to keep the OAM drawing routine simple.

I guess I can make the code slightly more readable by renaming the "temp" variables, but I don't know what I can do about the code size.
TrekkiesUnite118
Posts: 92
Joined: Fri Mar 08, 2013 5:56 pm

Re: How do I make my code more "modular"?

Post by TrekkiesUnite118 »

If it's really bothering you this much about what code he's referring to why not just ask him directly? He's on this forum as well as on discord. You could just message him directly. If you don't like that idea then another option is if there's nothing really secret or proprietary about your code you don't want others having access to you could always open it up to a few community members for a code review or something.
psycopathicteen
Posts: 3140
Joined: Wed May 19, 2010 6:12 pm

Re: How do I make my code more "modular"?

Post by psycopathicteen »

So tonight I went ahead and renamed all the temporary variables in the animation routine to whatever name describes how they are used in the code.

Code: Select all

define CHR_ROM_address({temp})
define CHR_ROM_bank({temp2})
define previous_sprite_index({temp3})
define metasprite_data_index({temp5})
define CHR_VRAM_address({temp6})
define vertical_sprite_run({temp7})
define horizontal_sprite_run({temp8})
define pixel_increment({temp10})
define attributes({temp11})
Temp4 is only used very briefly so I don't think I need to rename. I didn't end up using Temp9 at all for some reason.
Attachments
animation.txt
(12.78 KiB) Downloaded 36 times
TrekkiesUnite118
Posts: 92
Joined: Fri Mar 08, 2013 5:56 pm

Re: How do I make my code more "modular"?

Post by TrekkiesUnite118 »

So looking at what you posted I think I see a couple issues from just a readability standpoint. First formatting is rather poor. This makes it hard to read at times and important things like subroutine names, jump points, etc. don't stand out as clearly as they could which makes the code just kind of meld together into a blur. It kind of looks like what I'd expect to see in a debugger or Ghidra's disassembly window.

Next there's magic numbers everywhere. Yes some of these you have an explanation for up at the top, but when you're down around line 400 it's a pain to keep scrolling back up to remember what it was. If you can I'd make a lot of those magic numbers a constant or something so it's clear what they represent. It would also give the benefit of if you need to change that value, you only need to change it in one place rather than in multiple spots and risk missing one and introducing bugs. If a magic number is unavoidable, then a comment should be there on the line that clearly explains what it represents.

Finally it at least appears like there could be a lot of duplicate logic going on here. You seem to be doing a lot of similar logic for both small sprites and large sprites from finding slots in VRAM for them, copying them to VRAM, etc. Could any of that be consolidated? Again if the logic is similar there may be similar parts that someone may want to change later, and having a lot of that duplicated around would mean that's a lot more places you need to go make changes now.

I'm no expert in SNES Assembly so maybe some of this is unavoidable. But from just an outsider looking in trying to follow the code, those are the things that jump out at me the most. Sometimes a good thing to look at is how other projects structure their code. For example the Sonic games have been disassembled for years with very well commented and formatted code making it easy for different people to come in and get comfortable with it and start making changes to it. Those might not be a bad thing to look at to get an idea of how to just make code more readable and maintainable.
creaothceann
Posts: 610
Joined: Mon Jan 23, 2006 7:47 am
Location: Germany
Contact:

Re: How do I make my code more "modular"?

Post by creaothceann »

I'd use names for the registers, e.g. from anomie's regs.txt.

I'd indent all regular code (not the labels, defines, comment blocks) by 1 tab, and align comments that belong to the same section to the same column. Makes it much easier to find the labels. The current structure seems to mimic the indentation of high-level languages, but I'm not sure this works for assembly.

Instead of ...

Code: Select all

define CHR_ROM_address({temp})
define CHR_ROM_bank({temp2})
define previous_sprite_index({temp3})
define metasprite_data_index({temp5})
define CHR_VRAM_address({temp6})
define vertical_sprite_run({temp7})
define horizontal_sprite_run({temp8})
define pixel_increment({temp10})
define attributes({temp11})
... I'd align it like this:

Code: Select all

define CHR_ROM_address      ({temp  })
define CHR_ROM_bank         ({temp2 })
define previous_sprite_index({temp3 })
define metasprite_data_index({temp5 })
define CHR_VRAM_address     ({temp6 })
define   vertical_sprite_run({temp7 })
define horizontal_sprite_run({temp8 })
define pixel_increment      ({temp10})
define attributes           ({temp11})
Both the instructions and most identifiers (labels etc) are in lowercase. Instead of first_object_to_dma I'd use FirstObjectToDMA (personal preference).

If my assembler supports several instructions per line, I'd use it like this:

Code: Select all

	ldy.b {dma_updates};                   sta   {dma_destination}      , y
	;                         adc #$0100;  sta   {dma_destination_2}    , y
	;                         adc #$0100;  sta   {dma_destination}   + 8, y
	;                         adc #$0100;  sta   {dma_destination_2} + 8, y
	lda.b {CHR_ROM_address};               sta   {dma_address}          , y
	;                         adc #$0100;  sta   {dma_address}       + 8, y
	;                         adc #$0100;  sta.b {CHR_ROM_address}
	lda.b {CHR_ROM_bank};     ora #$8000;  sta   {dma_bank}             , y
	;                                      sta   {dma_bank}          + 8, y
	tya;  clc;                adc #$0010;  sta.b {dma_updates}

Code: Select all

+;  ora #$f0;  sta {vram_slot_flags}-$a00000,y;  lda vram_slot_lut_a,y;  ora #$02;  rep #$20;  and #$00ff;  asl;  rts  // shifting left clears carry
+;  ora #$0f;  sta {vram_slot_flags}-$a00000,y;  lda vram_slot_lut_a,y;  rep #$20;             and #$00ff;  asl;  rts  // shifting left clears carry
Attachments
animation2.txt
(13.81 KiB) Downloaded 18 times
My current setup:
Super Famicom ("2/1/3" SNS-CPU-GPM-02) → SCART → OSSC → StarTech USB3HDCAP → AmaRecTV 3.10
Pokun
Posts: 2675
Joined: Tue May 28, 2013 5:49 am
Location: Hokkaido, Japan

Re: How do I make my code more "modular"?

Post by Pokun »

Yeah macros can be used like subroutines without the overhead of the JSR/RTS. Though depending on how the assembler handles them they could be the cause of errors not happening in subroutines (much like the #define preprocessor directive in C), so they should be used with care if replacing subroutines.



I think many people here think it's fine to use register numbers as is without considering them magic numbers, but you should probably still leave a comment to explain what this does. Not for each line but for every few lines or so depending on how much you can explain in the comment.
If you want to give register numbers names, the official names Nintendo used are probably the most recognized ones:
INDISP = $2100, OBJSEL = $2101, BG1SC = $2107, NMITIMEN = $4200, WRIO = $4201 etc.

Hmm I think Anomie is using those too.


Other than the lack of comments, the other obvious thing I see is that you don't seem to use indention except in certain cases.
I agree with Creaothceann on this, normally assembly instructions and assembler directives are always indented while labels (including those used in equates) are never indented. Some assemblers doesn't check this, but many do and I think most people would prefer this way.
I've seen that a lot of people don't indent assembler directives though, only instructions. Are you using bass? That assembler tends to use pretty long directives ("origin" instead of ".org") making them more painful to indent, so I actually don't indent them in this particular assembler. Near also didn't seem to do that in his examples in the manual IIRC.

Some programmers use an extra level of indention for certain code to give it some hierarchy similar to high level languages. I don't do this myself, but I think it's fine if you want to do that.
Some people prefers to indent using a tab and others prefers 2 or more spaces, I do 2 spaces because I had bad experience with tabs not having a consistent length in different editors, messing up the format. I think 2 spaces is clear enough and it saves me from a bunch of extra space presses if I would use 4 or more. The only drawback I see is that I can't have a label on the same row as an instruction, but I just put it on an empty row before. Modern assemblers allows very long label names anyway which would be hard to fit if they had to be on the same line. A system that only allows short labels tends to make them harder to read with all the abbreviations and has a more limited namespace scope.

I also agree with Creaothceann that it is best to align defines and equates so they become easier to read.
Comments are also best to align so that they line up with each other on the same section of code.
I used to ignore this rule as a beginner but since I started doing that my code became much easier to read while not really costing as much extra time as I thought it would. I only ignore this rule in rare cases when there is an exceptionally long line of code (maybe a pretty long symbol name is used) that I really need a comment on that is too long.

Nowadays I also follow a rule to keep each line within 80 columns so that it can be displayed on anything that supports 80 columns, the typical standard for text terminals. Notepad++ which I use has an option for setting a vertical line to the left at a column of your choice to make this simpler.
Some people do this but I see that many don't and type longer lines. In C it is a bit harder to keep lines 80 columns due to the longer names of things (though C allows multi-line statements so it never seems to be a real problem), but assembly uses very short lines so it is easy enough.
creaothceann
Posts: 610
Joined: Mon Jan 23, 2006 7:47 am
Location: Germany
Contact:

Re: How do I make my code more "modular"?

Post by creaothceann »

I really like to put a lot of code on the screen, hence the several-instructions-per-line comment above. (Maybe it's because I started programming on 80x25 character displays - optionally 80x50, though that font didn't look too good - so reducing the number of lines is almost always a win for me.) There is usually a certain structure to an algorithm, and grouping related parts together to highlight these relationships makes it easier to read, even if it goes over 80 columns.

Generally speaking, what really helps in understanding an unfamiliar program is a high-level explanation of the problem, the algorithm used to solve the problem, and perhaps even previous attempts that turned out to be less optimal. This doesn't have to be preceding every function, but could be at the top of the source file (instead of the full page of copyright statements that is so symptomatic of OSS code) or in a text file in a directory that groups several source files together. Writing such a short, concise outline usually helps me concentrate when writing the actual code.

Regarding the thread topic: In the provided code snippet there's not really much to say about making it modular, because it's code that is bound pretty tightly to the data format listed at the top. Perhaps it could use a list of variables and other definitions that it expects to be present, or maybe the entire thing could be packed into a macro with a clearly defined list of arguments. In other words, making it more a "module" that is conceptually separate from the rest of the program, can be easily copied to a new project, and called somewhat like a black box.
My current setup:
Super Famicom ("2/1/3" SNS-CPU-GPM-02) → SCART → OSSC → StarTech USB3HDCAP → AmaRecTV 3.10
Fiskbit
Posts: 890
Joined: Sat Nov 18, 2017 9:15 pm

Re: How do I make my code more "modular"?

Post by Fiskbit »

Note that code formatting is a personal thing and people have very different styles that work for them. For me, onscreen code density is a negative; I want one instruction per line and I use vertical whitespace (two line breaks) to separate blocks of code (usually 2-6 lines) that each do one thing. You should find a style you like that works for you, but by that I'm not suggesting that just anything is a suitable style. The decisions should be deliberate with the goal of making the code as easy to understand at a glance as possible and to minimize the risk of bugs as the code changes. Assume you'll go years without touching the code such that you lose most familiarity with it.

That said, I agree with much of the general input you're getting here:
- Indentation to distinguish between label definitions (and sometimes directives) vs code is really helpful. Some people also use indentation to show loops, or depth in an if/else style section of code.
- Constants really should be named labels where reasonable. It's easy to go overboard with this; you don't necessarily need names for literally every number you use, but naming them is a form of documentation and ensures that if the number needs to change, it changes everywhere without the risk of forgetting some place.
- Naming registers helps, too. As the number of registers grows (and SNES has a lot of them!), it becomes harder and harder to remember all the numbers and easier to make a mistake. Names reduce risk of bugs and are another form of documentation where it's easier to tell at a glance what code is doing.
psycopathicteen
Posts: 3140
Joined: Wed May 19, 2010 6:12 pm

Re: How do I make my code more "modular"?

Post by psycopathicteen »

Do you guys think this is an improvement over the last version? I tried to make it a little more obvious where the branches go by indenting the code that gets branched over. I also replaced the different versions of the DMA set-up code with a macro, but I noticed that this little change is making slowdown happen during the Robo-Seahorse fight that wasn't there before.
Attachments
animation.txt
(13.06 KiB) Downloaded 35 times
calima
Posts: 1745
Joined: Tue Oct 06, 2015 10:16 am

Re: How do I make my code more "modular"?

Post by calima »

It looks fine, though I'd give names to the loop labels if they're farther than a couple lines.
Post Reply