Tips and Guidelines
Here's some tips and guidelines when contributing to the MiSTer project.
Indent with tabs, not spaces
This may be a controversial subject to some, however there is good reason to indent with tabs instead of spaces in this instance. The project is uploaded to github and spaces are not handled that well by github in terms of diff compares, git blame, formatting, etc... Tabs are more compatible and will be easier for everyone to work with.
Comment your code when appropriate
It may take some extra time, but it saves everyone, including yourself, time in the long run. Make sure to be relatively thorough in commenting your code. For example, a good example of commented code from the Atari7800 core in TIA.sv starting at line 786:
module playfield ( input clk, // Master clock input reset, // System reset input logic clkp, // Pixel Clock input clock_t hclk, // Horizontal clock phase 2 input reflect, // Control playfield, 1 makes right half mirror image input cnt, // center signal, high means right half input rhb, // Reset HBlank signal input [19:0] pfc, // Combined playfield registers output logic pf // Playfield graphics );
When instantiating this module, this dev added a comment to the end of all of her Inputs and Outputs. One shouldn't assume everyone else knows that
Horizontal clock phase 2 even if the name makes sense to yourself. Other good examples of good comments are separating sections of assignments or port instantiations by category, or explaining the "Why" behind a particularly unintuitive section of code.
However there is such a thing as commenting your code too much. This should also be avoided. Here is an example:
It's obvious that this is a color look up table because the wire is named
color_lut so that doesn't need to be described. It's obvious that
TRANSP_DETECT is for transparency detection.
For an extreme and deliberately funny example:
////////////////////////////////// /// ~~~~~~~~~~~~~~~~~~~~~~~~~~ /// /// REGION PRIORITY HANDLING /// /// ~~~~~~~~~~~~~~~~~~~~~~~~~~ /// ////////////////////////////////// case(status[28:27]) // Status Bits 27 through 28 // Region priority is in CONF_STR section at line 256 of this file // If line 256 is no longer valid then here is a reference to what it says: // "D2ORS,Priority,US>EU>JP,EU>US>JP,US>JP>EU,JP>US>EU;", // O = Option, R and S are the status bits. // 2 status bits means there are 4 combinations of options // Case statements for this begin with 0 and end in 3 // Since there were two switches and 4 options as explained above you can treat 0-3 as 1-4 if you'd like // First Case Statement (0 is actually the first one) 0: if(hdr_u) region_req <= 1; // NTSC-U Region else if(hdr_e) region_req <= 2; // PAL Region else if(hdr_j) region_req <= 0; // NTSC-J Region else region_req <= 1; // Set to NTSC-U if there is no region in the header // Second Case Statement (1 is actually the second one) 1: if(hdr_e) region_req <= 2; // PAL Region else if(hdr_u) region_req <= 1; // NTSC-U Region else if(hdr_j) region_req <= 0; // NTSC-J Region else region_req <= 2; // Set to PAL if there is no region in the header // Third Case Statement (2 is actually the third one) 2: if(hdr_u) region_req <= 1; // NTSC-U Region else if(hdr_j) region_req <= 0; // NTSC-J Region else if(hdr_e) region_req <= 2; // PAL Region else region_req <= 1; // Set to NTSC-U if there is region in the header // Fourth Case Statement (3 is actually the fourth one) 3: if(hdr_j) region_req <= 0; // NTSC-J Region else if(hdr_u) region_req <= 1; // NTSC-U Region else if(hdr_e) region_req <= 2; // PAL Region else region_req <= 0; // Set to NTSC-U if there is region in the header endcase // End of the case statement // Okay I can put things that aren't in the case statement down here ////////////////////////////////////////////// /// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /// /// END REGION OF REGION PRIORITY HANDLING /// /// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /// //////////////////////////////////////////////
Please don't do things like this.
Align ends, assignments, and proper indentation
Try to keep similar elements in our code aligned for readability purposes, for example, in the EEPROM_24C0x.sv file from NES_MiSTer, this:
...is a lot easier to read than this:
Sure, they both do the same thing, but it was much easier to parse what each thing did quickly. SystemVerilog is not a strongly typed language with regards to indentations, so you won't get compilation errors if you fail to indent your ends accurately. But we should indent them appropriately anyways.
is much easier to read than:
Both will succesfully compile and be functionally identical, but it takes a little longer for our mind to parse on first glance. Aligning comments at the end of a line is also helpful in improving readability as shown in the
playfield module example near the top of this page.
Naming things to be easily understood
In EEPROM_STM95 in the Genesis core, this module's port instantiation uses
_n to signify that the signal is "active low":
It's also commented, but now you know after reading this that anytime a signal with
_n is used, that is an active low signal, it doesn't need to be described as active low in a comment each time it is used from then on.
If you run out of DSP or Logic space accidentally
If you run out of space in compilation, you might be accidentally implying your code to be put into DSP, which is very limited. You can specify that the compiler use logic instead by using the multstyle:
This will force this module into logic space instead of it ever using DSP or block ram. Also you can force a module into DSP the same way with
(* multstyle = "dsp" *). The
multstyle attribute can be used for Module declarations, variable declarations, and binary expressions.