[fuse-emulator-devel] [ fuse-emulator-Patches-2886045 ] Spec SE 64 Colour Mode

[SourceForge.net <no...@so...>]

Patches item #2886045, was opened at 2009-10-25 18:47
Message generated for change (Comment added) made by quazatr0n
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=596650&aid=2886045&group_id=91293

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Gilberto Almeida (quazatr0n)
Assigned to: Nobody/Anonymous (nobody)
Summary: Spec SE 64 Colour Mode

Initial Comment:
- 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.

----------------------------------------------------------------------

Comment By: Gilberto Almeida (quazatr0n)
Date: 2009-10-26 00:45

Message:
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.

----------------------------------------------------------------------

Comment By: Philip Kendall (pak21)
Date: 2009-10-25 21:47

Message:
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.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=596650&aid=2886045&group_id=91293