[FraGag]

I'd like to suggest what I consider an "improvement" to declare the labels for the RAM variables. I just tested this and I was quite surprised that it worked, but before making the change, I want to get some opinions. Here's how the last few RAM variables are currently declared in the disassembly:

Syntax Highlighted Code: ASM

Demo_mode_flag =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF0[/color] ) [color= #adadad; font-style: italic;]; 1 if a demo is playing (2 bytes)[/color]

Demo_number =			ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF2[/color] ) [color= #adadad; font-style: italic;]; which demo will play next (2 bytes)[/color]

Ending_demo_number =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF4[/color] ) [color= #adadad; font-style: italic;]; zone for the ending demos (2 bytes, unused)[/color]

Graphics_Flags =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF8[/color] ) [color= #adadad; font-style: italic;]; misc. bitfield[/color]

Debug_mode_flag =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFFA[/color] ) [color= #adadad; font-style: italic;]; (2 bytes)[/color]

Checksum_fourcc =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFFC[/color] ) [color= #adadad; font-style: italic;]; (4 bytes)[/color]

And here's another way to declare them:

Syntax Highlighted Code: ASM

	phase	[color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF0[/color]

Demo_mode_flag:			ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; 1 if a demo is playing (2 bytes)[/color]

Demo_number:			ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; which demo will play next (2 bytes)[/color]

Ending_demo_number:		ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; zone for the ending demos (2 bytes, unused)[/color]

				ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color]

Graphics_Flags:			ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; misc. bitfield[/color]

Debug_mode_flag:		ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; (2 bytes)[/color]

Checksum_fourcc:		ds.[color= #00bfff;]l[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; (4 bytes)[/color]

	dephase                       

	![color= #00CC66;]org[/color]	[color= #ff0000;]0[/color]

Notice how the unlabelled variable is easily noticeable. I believe this method would be better because we don't have to type the addresses for every variable. Also, it makes it easy to give several labels to the same location (this would be useful for *_End labels, amongst others).

However, I only tested this as well as Chunk_Table and Level_Layout. I don't quite understand the problem ramaddr tries to solve, but if this method makes it happen again, then we won't be able to use it.

[shobiz]

Puto, on Oct 20 2008, 12:19 AM, said:

Or you could put it in a separate file, and in build.bat, assemble it separately, compress it, and incbin/binclude it.

Yeah, that'd work perfectly for this case, dunno why I didn't think of that. Obviously the sound driver needed to reference addresses in the main ASM file so it couldn't be used for that though.

FraGag, on Oct 20 2008, 09:32 AM, said:

I'd like to suggest what I consider an "improvement" to declare the labels for the RAM variables. I just tested this and I was quite surprised that it worked, but before making the change, I want to get some opinions. Here's how the last few RAM variables are currently declared in the disassembly:

Syntax Highlighted Code: ASM

Demo_mode_flag =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF0[/color] ) [color= #adadad; font-style: italic;]; 1 if a demo is playing (2 bytes)[/color]

Demo_number =			ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF2[/color] ) [color= #adadad; font-style: italic;]; which demo will play next (2 bytes)[/color]

Ending_demo_number =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF4[/color] ) [color= #adadad; font-style: italic;]; zone for the ending demos (2 bytes, unused)[/color]

Graphics_Flags =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF8[/color] ) [color= #adadad; font-style: italic;]; misc. bitfield[/color]

Debug_mode_flag =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFFA[/color] ) [color= #adadad; font-style: italic;]; (2 bytes)[/color]

Checksum_fourcc =		ramaddr( [color= #ff0000;]$[/color][color= #ff0000;]FFFFFFFC[/color] ) [color= #adadad; font-style: italic;]; (4 bytes)[/color]

And here's another way to declare them:

Syntax Highlighted Code: ASM

	phase	[color= #ff0000;]$[/color][color= #ff0000;]FFFFFFF0[/color]

Demo_mode_flag:			ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; 1 if a demo is playing (2 bytes)[/color]

Demo_number:			ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; which demo will play next (2 bytes)[/color]

Ending_demo_number:		ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; zone for the ending demos (2 bytes, unused)[/color]

				ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color]

Graphics_Flags:			ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; misc. bitfield[/color]

Debug_mode_flag:		ds.[color= #00bfff;]w[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; (2 bytes)[/color]

Checksum_fourcc:		ds.[color= #00bfff;]l[/color]	[color= #ff0000;]1[/color] [color= #adadad; font-style: italic;]; (4 bytes)[/color]

	dephase                       

	![color= #00CC66;]org[/color]	[color= #ff0000;]0[/color]

Notice how the unlabelled variable is easily noticeable. I believe this method would be better because we don't have to type the addresses for every variable. Also, it makes it easy to give several labels to the same location (this would be useful for *_End labels, amongst others).

However, I only tested this as well as Chunk_Table and Level_Layout. I don't quite understand the problem ramaddr tries to solve, but if this method makes it happen again, then we won't be able to use it.

Eh, I don't really like it, it makes it less obvious what the real address is (although it does enable the size of the variable to be read off at a glance). About ramaddr, it's used for stuff like this:

Syntax Highlighted Code: ASM

	[color= #00bfff;]cmp[/color].[color= #00bfff;]w[/color]	[color= #ff0000;]#[/color]MainCharacter,a0

Without ramaddr, MainCharacter would be taken as $FFFFB000 and AS would complain about it not fitting in a word (you'd have to use MainCharacter&$FFFF instead). With ramaddr, the equate gets internally converted to -$5000 (I know $FFFFB000 and -$5000 are exactly the same thing in 32-bit mode, but AS isn't smart enough to figure that out apparently), so AS can easily shrink that into a word, and you don't need to AND by $FFFF each time you want to use the word-sized form of a RAM equate.

[Sik]

Woah, AS failure o_o;

Anyways, I guess that works better for new programs rather than for disassemblies that need to be exact =P

[FraGag]

shobiz, on Oct 20 2008, 04:19 AM, said:

Eh, I don't really like it, it makes it less obvious what the real address is (although it does enable the size of the variable to be read off at a glance).

I thought I could add a comment about how to use the MESSAGE directive to show the address of a variable at build-time. As for unlabelled variables, we should put the address in a comment, because it's most likely they're the ones we'll be looking for.

shobiz, on Oct 20 2008, 04:19 AM, said:

About ramaddr, it's used for stuff like this:

Syntax Highlighted Code: ASM

	[color= #00bfff;]cmp[/color].[color= #00bfff;]w[/color]	[color= #ff0000;]#[/color]MainCharacter,a0

Without ramaddr, MainCharacter would be taken as $FFFFB000 and AS would complain about it not fitting in a word (you'd have to use MainCharacter&$FFFF instead). With ramaddr, the equate gets internally converted to -$5000 (I know $FFFFB000 and -$5000 are exactly the same thing in 32-bit mode, but AS isn't smart enough to figure that out apparently), so AS can easily shrink that into a word, and you don't need to AND by $FFFF each time you want to use the word-sized form of a RAM equate.

I think AS can use 64-bit integers internally, so that would explain why it doesn't work. We would either have to use a negative number (and only this number would be written as a negative number) or a 64-bit number (probably not portable, and not very intuitive) with PHASE, unless ramaddr itself works with PHASE (and I can't see why it wouldn't).

Syntax Highlighted Code: ASM

	phase	ramaddr([color= #ff0000;]$[/color][color= #ff0000;]FFFF0000[/color])

This post has been edited by FraGag: 21 October 2008 - 07:11 PM

[FraGag]

Double post, because I really want this stuff in. I just tested with those problematic variables: using the current equates, I tried to remove the use of ramaddr in Object_RAM and it gave me errors (as I expected). Using phase with or without using ramaddr works. I'll leave it anyway, in case the problem reappears on other systems.

[shobiz]

Looking at it again it's sorta grown on me (well technically IDA does the exact same thing in a RAM segment but the addresses are prefixed to each line so I didn't notice that much), so I'm cool with having it in. The only real issue I can see with it is that it makes relocating a single variable harder, although on the other hand it makes swapping variable blocks (as was done in S&K for a few variables) much easier.

[MoDule]

I just tried to commit the work I did on the Monitor code and there's a conflict in s2.asm, s2.constants.asm and changelog.txt. What should I do?

[FraGag]

MoDule, on Oct 25 2008, 06:32 PM, said:

I just tried to commit the work I did on the Monitor code and there's a conflict in s2.asm, s2.constants.asm and changelog.txt. What should I do?

If you're using TortoiseSVN, read section 5.6 (Daily Use Guide > Resolving Conflicts) of the help file (accessible from the Start Menu). You may also want to read TortoiseMerge's help file if you don't understand how it works. Don't forget to mark the conflict as resolved when you're done.

[Hivebrain]

It might be a good idea to split s2.asm up, to help avoid this kind of thing (also to improve readability).

[FraGag]

I've been thinking about splitting that file, but that won't really help fix the conflicts, as they usually happen when 2 or more people changed the same line(s).

[MoDule]

There, I fixed the conflicts. I hope I didn't undo anyone's work in the process. It doesn't look like it, but there were a lot of small differences.

[shobiz]

To quote changelog.txt,

* The major change is the addition and usage of the macros zoneOffsetTable, zoneTableEntry and
  zoneTableEnd. These macros automatically sort offset tables which use the zone ID as an offset, and thus
  enable easy re-arranging of zone IDs without having to worry about re-arranging dozens of offset tables as
  well. Additionally, whenever a new zone is added, the zoneTableEnd macro will automatically warn you about
  which offset tables you need to add more stuff to. I've tested out some swaps and everything seems to work
  properly, but I'd appreciate it if people could test different combinations out, and either fix any bugs
  encountered themselves or post about them for others to fix. It should be noted that useFullWaterTables should
  be set to 1 inside Assembly Options when you're shifting IDs around, and that s2p2bin will complain about
  overlapping allocations, but these warnings can be safely ignored.

I know annotative additions are probably far more useful from a practical perspective, but since FraGag had already set up his awesome dynamic IDs system this seemed a logical extension. As I said above though, I've only tested out a few swaps so far and haven't done very thorough testing, so it'd be useful if others could try changing IDs around and either fix anything which breaks or report it for fixing. The most likely causes of errors would be assumptions the programmers made (e.g. there would be no zone with water with an ID below $8, for which I had to add the useFullWaterTables option, and that EHZ would always be the first zone, for which a number of if/else statements were needed), and zone references which haven't been converted to equate form yet.

[FraGag]

Wow, that's really cool. That's basically what I had in mind, and you executed it brilliantly! Good job!

[FraGag]

Alright, I've been really busy lately (and I think I'm not alone ), but I want to make another suggestion. Basically, the CC and CS condition codes have equivalents, respectively HS (typoed as HI in the official documentation) and LO. I've often met BCC and BCS instructions that had me wondering what they did (I used to mix them up, and I probably still do), and I think it would be better to replace some instances of BCC and BCS with BHS and BLO (e.g. after CMP instructions). I feel they are the natural complements to HI and LS, which don't have equivalents.

[shobiz]

I agree, it would make the meaning of the branches clearer.