#209 Spec SE 64 Colour Mode

open
nobody
None
5
2013-01-01
2009-10-25
Gilberto Almeida
No

- Initial patch to add a 64 colour mode to Spec SE, according tothis specification: http://scratchpad.wikia.com/wiki/ZX_Spectrum_64_Colour_Mode
+ Added a 64 byte CLUT
+ Added a couple of I/O ports to read/write the CLUT values and enable/disable the 64 colour mode.

Related

Patches: #323

Discussion

  • Philip Kendall
    Philip Kendall
    2009-10-25

    Technically, I don't see anything wrong with this, but the coding style needs work. See hacking/coding_style.txt for some of this, but largely it's a case of following what the rest of the source does.

    * Inconsistent use of spaces (eg se_clut_curr_reg = 0 vs se_clut_palette_change=0; other examples exist)
    * The clut_* functions should be static unless they're being called from elsewhere
    * All parameter in prototypes should have names
    * All hex constants should be in lower case (0xffff vs 0xBF3B)
    * No leading asterisk on continuations of comment lines
    * In a function definition, the name should start on a new line
    * 2 character indents
    * The opening brace of a function definition should be on a new line
    * No parentheses around simple return statements
    * if( condition ), not if(condition)
    * Spaces around condition operators: se_clut_curr_reg >= 0, not se_clut_curr_reg>=0

    Other bits:

    * There's no point having >= 0 checks on unsigned variables. That's always true.
    * Missing else in clut_dataport_read
    * You should probably be checking in clut_dataport_write that the value has actually changed before setting se_clut_palette_change.

     
  • Thank you for the feedback, Phillip.
    I reworked the patch to address the issues you pointed out.
    I think this version is more in line with the project's coding style.