Nes C - Char comparison returning True when clearly not

[team_disposable]

I'm trying to compare two unsigned char variables in an if statement.
For some reason, even when I hard code the two values to be different, the if statement keeps returning true.

Clearly I'm doing something wrong here - would anyone be able to point out what?

Originally I was trying to pull a value from an array and compare it:

So I would declare and set my array and my check digit:

Code: Select all

unsigned char enemy_onscreen_value = 1;
unsigned char enemy_onscreen_check[] = {
0,0,0,0
};

And then later, in a function, compare the two:

Code: Select all

void update_Enemies(void) {
	
	for(index2 = 0; index2 < sizeof(enemy_onscreen_check); ++index2){
		
		if(enemy_onscreen_check[index2] >= enemy_onscreen_value) {
                 	update_Enemy_Sprite(index2);
					
		}
	
	}
		
} 

These seems to return true for all four values in the array, even though all of them are zero! It does the same if i set it to ==.

I then tested with just the if statement:

Code: Select all

unsigned char enemy_onscreen_value = 1;
unsigned char enemy_onscreen_1 = 0;

void update_Enemies(void) {
	if(enemy_onscreen_value == enemy_onscreen_1) {
		update_Enemy_Sprite(index2);				
	}
}

Both of these run update_Enemy_Sprite, even though both should be false.

I can find nothing on the internet that says I am doing this incorrectly. Where am I going wrong?

Thanks,

[lidnariq]

CC65 emits assembly output. Maybe try checking to see if it's doing something vaguely correct?

[Kasumi]

Not too familiar with the cc65 setup, but just out of curiosity, what happens when you use const?

Code: Select all

const unsigned char enemy_onscreen_value = 1;

Also curious, if you reverse the condition, does it still do the same thing? Just throwing out things I'd try based on what's here, but checking assembly output would be best.

Especially in the second example, I'd wonder if something else is modifying it, or if update_Enemy_Sprite is just run elsewhere.

[koitsu]

Does the behaviour change if you use int rather than char?

[dullahan]

Bummer. Two questions:

1. How are you invoking the compiler? Specifically are you using any optimizer flags (-O, -Oi, etc...)? The optimizer sometimes has issues with comparisons; e.g. https://github.com/cc65/cc65/issues/167

2. What version of CC65 are you running?

[team_disposable]

These were all immensely helpful, thank you.

I checked the assembly code being made. There were no problems with the for loop (code below).

I applied a const to the declaration of the array, and then everything seemed to work! The only visible change in the assembly is that it has moved the decleration of the array from the DATA section to the RODATA section.

The "IF statement now also works with a hardcoded 1,, which it didn't before:

Code: Select all

	if(enemy_onscreen_check[index2] == 1) {

Does anyone have any ideas why the move from DATA to RODATA changes this? I intended to use the array as alive or dead flags and change them based on the enemies status, so would prefer them to be in RAM. Am I overwriting them somehow? This might be a stupid question, but can I set that piece of ram as protected, or assign it to a space?

In answer to previous questions:

1. Ints don't seem to make a difference
2. I'm using CC 2.15
3. reversing the condition is broken without the const, works with the const

Here is the assembly:
Declaration of array. With const this is in RODATA, without it is in DATA:

Code: Select all

_enemy_onscreen_check:
	.byte	$01
	.byte	$00
	.byte	$01
	.byte	$00

Here is the if statement:

Code: Select all

;
; for(index2 = 0; index2 < sizeof(enemy_onscreen_check); ++index2){
;
	lda     #$00
	sta     _index2
L054E:	lda     _index2
	cmp     #$04
	bcs     L03FE
;
; if(enemy_onscreen_check[index2] == 1) {
;
	ldy     _index2
	lda     _enemy_onscreen_check,y
	cmp     #$01
	bne     L054F
;
; update_Enemy_Sprite(index2);
;
	lda     _index2
	ldx     #$00
	jsr     _update_Enemy_Sprite
;
; for(index2 = 0; index2 < sizeof(enemy_onscreen_check); ++index2){
;
L054F:	inc     _index2
	jmp     L054E
;
; }
;
L03FE:	rts

For anyone else reading this who is just starting out, I found an excellent explanation of the different assembly functions here:

https://www.c64-wiki.com/index.php/CMP

Thanks for the help, that was driving me crazy!

[rainwarrior]

My guess would be that the DATA segment is not getting initialized on startup. The CRT should do this for you before calling main, but you could be bypassing that somehow.

If you'd run it in a debugger and put a breakpoint on writes to that variable, you'd find out if and when it's getting initialized, or if it's getting overwritten later by something else misbehaving, etc.

[team_disposable]

Thanks, Rainwarrior.

I think you're right! Looking at the assembly, every var in the DATA section is something I'm not yet using, and trying to reference anything else in there gets the same result.

I managed to set a breakpoint using Dougeffs excellent guides, but wasn't too sure what I was looking at. Instead, I added a few assignments to the very beginning of main, before anything else ran. The comparison then worked.

How would I initialize the data section? I'm presuming this would be part of nes.cfg?

I have this line in there already:

Code: Select all

  DATA:     load = PRG, run = RAM, type = rw,  define = yes;
 

[rainwarrior]

In CC65 it would normally be initialized by the subroutine "copydata" found in the CRT before it calls main.
https://github.com/cc65/cc65/blob/maste ... copydata.s

[thefox]

Dougeff's init code (reset.s) indeed doesn't seem to be calling copydata.

While it's a good idea to use copydata to restore the standard C behavior, alternatively you can simply make sure to initialize the global variables in your own code before using them.

[dougeff]

crt0.s has...

Code: Select all

JSR	zerobss		; Clear BSS segment.
JSR	copydata	; Initialize DATA segment.
JSR	initlib		; Run constructors.

Which I have omitted, when I rewrote this. Should I have included them?

[rainwarrior]

While it's a good idea to use copydata to restore the standard C behavior, alternatively you can simply make sure to initialize the global variables in your own code before using them.

It's a standard C language feature, and the C compiler depends on it. Omitting it silently breaks your code, as we've seen. It has no way of knowing that you've removed that essential step in the CRT init, so there's no way it can produce an error to prevent you from using the feature.

I don't think it's merely a good idea. The alternative is disabling basic C language stuff for no good reason while leaving yourself prone to silent errors.

Code: Select all

JSR	zerobss		; Clear BSS segment.
JSR	copydata	; Initialize DATA segment.
JSR	initlib		; Run constructors.

Which I have omitted, when I rewrote this. Should I have included them?

zerobss just sets all RAM to 0. I don't think it's strictly necessary, but it does provide a consistent startup state. (You may already be doing this yourself, if you follow common suggestions from the NESDev community about startup code.)

copydata, or something that duplicates it is essential.

initlib is only needed if you use the constructor/destructor feature of CC65. It's a nonstandard extension of C that you probably don't need (not related to C++ constructors either). Some of its library stuff uses that feature but you don't necessarily need to include those things, and if your linker CFG doesn't have the CONDES feature specified, you'll get an error preventing you from accidentally using it. (Library modules are lazily linked on demand, so it's possible to have sleeper modules in the library that need CONDES that you haven't noticed cause you just haven't used them. Simple to find them with a text search though.)

Info on CC65 constructors:
Feature explanation: http://cc65.github.io/doc/ca65.html#s16
Linker config setup: http://cc65.github.io/doc/ld65.html#ss5.9

(As I said, though, there's little harm in just ignoring the CONDES feature. You'll get an error if you do anything wrong with this by accident, as long as you didn't use the CONDES feature in your linker CFG.)

[thefox]

While it's a good idea to use copydata to restore the standard C behavior, alternatively you can simply make sure to initialize the global variables in your own code before using them.

It's a standard C language feature, and the C compiler depends on it. Omitting it silently breaks your code, as we've seen. It has no way of knowing that you've removed that essential step in the CRT init, so there's no way it can produce an error to prevent you from using the feature.

There's one way to get warnings -- set the DATA segment type to "bss", linker should then give a warning when initialized data is placed in the segment.

Anyway, I moreso meant to present manual initialization as a quick hack rather than a real alternative for fixing the initialization code. I could've been more clear about that.

[rainwarrior]

Ah, yes you could intentionally disable the DATA segment. Heh.

I was looking at dougeff's reset.s (found in lesson1.zip here) and aside from the missing copydata, I noticed it tries to initialize sprites via $4014 during init. This will work on most emulators, but on a real NES any data you upload to OAM there is going to decay before you turn rendering on, so whatever you upload will end up basically random on the real thing. $4014 is normally only useful if done within vblank immediately before a rendered frame.