glitches on Apple II about cc65-chess HOT 26 CLOSED

Thank you @iss000 - that does indeed fex the problem. I opted for the line: inv = blackWhite ^ !((piece & PIECE_WHITE) != 0); simply because I would never think to use !!

from cc65-chess.

I can confirm that the issue does indeed now happen. With optsize as decsribed and with optspeed, when drawing the board and it gets to the garbled pieces, the game will crash. I can't look into it right now to make sure this is the issue but I when I look at a version that I previously built, and that does work, it is 24976 bytes. The version I built with a newer compiler (cl65 V2.19 - Git 1093d16 - mabe a month or two ago?) generated an executable of 25961 bytes. I thus suspect a compiler change.

When I have time, I will investigate further (I always kept the previous compiler and just with the last time I updated I didn't - now I sure wish I had ;)

from cc65-chess.

Hello
I was able to build it with a previous commit of cc65 ( 2a421396, 2019/11/29 )

the reason is still unclear, but it would be ok with me 😀

from cc65-chess.

• The change in the binary file size is no surprise. If you take a look at the cc65 change log, you'll notice that the compiler basically changes all the time.
• Needless to say that it would be great to bisect this regression. If it is a cc65 problem it would of course be desirable to have a cc65 issue opened. When being able to pinpoint the cc65 change that introduced it, I guess it wouldn't be necessary to additionally create a minimal reproduction program.
• I think for everybody reading this it would be great if a small set of moves were given that trigger the issue.

from cc65-chess.

The issue appears when mixing XOR and the result from logical operation.
The fix changes from:
inv = blackWhite ^ (piece & PIECE_WHITE) != 0;
to
inv = blackWhite ^ (!!(piece & PIECE_WHITE) != 0);
in src/apple2/platA2.c (line 218)

I'm using latest cc65 build from sources.

EDIT: I didn't checked if other platforms are affected.

from cc65-chess.

my build also works fine! 😄 thank you !!

from cc65-chess.

@iss000: Am I right to presume that your recent comment is no answer to my question but rather refers to @StewBC's code?

from cc65-chess.

@colinleroy Thanks for taking care of that! It's actually a nice show case for the usefulness of actually communicating with the cc65 team 😃

from cc65-chess.

So bad is today and good was Apr 19, 2023 but bisect says first bad commit was 2021? This was my first time using bisect. I suppose that the 2021 commits didn't get merged till after Apr 2023? I don't know how to look to see and my first attempts at finding the asnwers in Google didn't work out - and now I am out of time.

You suppose correctly, the PR was opened in October 2023 with commits two years old :)

For what it's worth, you can find the history with gitk, searching for the commit hash, it'll mention when it got merged:

from cc65-chess.

@colinleroy Thank you! I appreciate you sorting this. It still sat in my inbox as something I needed to do when I finished what I am busy doing :)
Now I'll fix the chess commit when I am finished doing what I am busy doing ;)
Anyway, much appreciated!

from cc65-chess.

@oliverschmidt could you give me some advices?

from cc65-chess.

No good idea :-(

Just some random ideas:

• Maybe the code makes assumptions which aren't met on the emulator used. I'd try the same binary with i.e. AppleWin.
• Maybe cc65 has changed something over time. I'd try to build with a cc65 HEAD around the time the Apple II port was done.

from cc65-chess.

From Oliver's list:

1. ACK

2. I did this bisect now - here is the result - The first bad commit:
   commit 8b6d78a075dafde8583c4992d37995d9b4c9db97
   Author: acqn [email protected]
   Date: Mon Mar 1 22:11:22 2021 +0800

   Added Opt_a_tosbitwise for 8-bit bitwise operations.

src/cc65/coptstop.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)

But this doesn't make sense to me because I started with head today as bad and as good I used:
commit e5f4ca6b898bf581751901fd5792764adfb7730a
Merge: 128b15a71 1f9594560
Author: Bob Andrews [email protected]
Date: Wed Apr 19 15:07:16 2023 +0200

Merge pull request #2058 from icepic/patch-1

Update lynxsprite.c

So bad is today and good was Apr 19, 2023 but bisect says first bad commit was 2021? This was my first time using bisect. I suppose that the 2021 commits didn't get merged till after Apr 2023? I don't know how to look to see and my first attempts at finding the asnwers in Google didn't work out - and now I am out of time.

3. Simply running the game and pressing a key on the title screen advances to the board. The bottom (white) pieces stick out above the menu. These pieces can be seen to be corrupt.

from cc65-chess.

Pieces are broken since PR 2240

OK
bb1b5c36 Bob Andrews [email protected] on 2023/10/18 at 19:51
OK
Merge pull request #2238 from acqn/BoolOptFix
8e6c0c14 Bob Andrews [email protected] on 2023/10/26 at 20:06
ERROR
Merge pull request #2240 from acqn/BitwiseOpt
a8d00bfa Bob Andrews [email protected] on 2023/10/26 at 23:53
ERROR
316ae886 Bob Andrews [email protected] on 2023/10/26 at 23:56

disks I tested are here.
disk2.zip

from cc65-chess.

I built them on Intel Mac.

> gcc --version
Apple clang version 15.0.0 (clang-1500.1.0.2.5)
Target: x86_64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

from cc65-chess.

Can you please check if #12 makes a difference?

from cc65-chess.

still problem

build with cc65 HEAD 2024/01/09 at 7:02

• cc65-Chess_patch-1_cc65_HEAD_ERR.dsk - broken

build with cc65 8e6c0c14 2023/10/26 at 20:06

• cc65-Chess_patch-1_cc65_8e6c0c14_OK.dsk - OK

cc65's differences between 8e6c0c14 (makes OK build) and a8d00bfa (makes error build)
I hope this can be a hint.

patch-1_builds.zip

from cc65-chess.

Sorry for being slow on the take-up, but isn't (piece & PIECE_WHITE) != 0 already a boolean expression? Can someone please explain what the two changes (the one proposed by @iss000 and the one @StewBC opted for) are doing?

from cc65-chess.

It's an order of precedence issue. The orifinal code makes inv == 128 and the new code makes it == 1 if piece = 0 and PIECE_WHITE == 128. AT least, I think that's what's going on. Empirically tested I can say that inv == 128 and not 1 in the old vs new code.

from cc65-chess.

Thanks for the feedback. Sorry for not being precise enough: I'm wondering why it is that way.

from cc65-chess.

Just for the record: my favorites actually are:
inv = blackWhite ^ (piece & PIECE_WHITE ? 1:0);
or
inv = blackWhite ^ ((piece>>7)&1);
Assuming that '!' returns always '0' or '1' isn't always portable.
There are compilers which return '0' or '-1' ... 😄

from cc65-chess.

This code below produces, on MSVC and gcc, the output:
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0
but on the Apple II it produces:

If I go to an "old working" version of cc65, I get the same output as in MSVC and gcc from cc65. I don't know what the standards say about any of this, but cc65 has changed its behaviour.

This code was used on the Apple II but with cstdio and cprintf, and with a while(!cgetc()){;} at the end of main so I could see the output.

#include <stdio.h>

#define PIECE_WHITE 128
unsigned char gChessBoard[8][8];

void plat_DrawSquare(char position)
{
    char inv;
    char y = position / 8, x = position & 7;
    char piece = gChessBoard[y][x];
    char blackWhite = !((x & 1) ^ (y & 1));

    if(piece) {
        inv = blackWhite ^ (piece & PIECE_WHITE) != 0;
    } else {
        inv = 0;
    }
    if(!(x%8)) {
        printf("\n\r");
    }
    printf("%0d ", inv);
}

void main() {
    int i;
    for(i = 0; i < 16; i++) {
        gChessBoard[i/8][i%8] = 0;
        gChessBoard[6+i/8][i%8] = PIECE_WHITE;
    }
    for(i = 0; i < 16; i++) {
        plat_DrawSquare(i);
    }
    for(i = 0; i < 16; i++) {
        plat_DrawSquare(6*8+i);
    }
}

from cc65-chess.

Just for the record:

• cgetc() waits for a keyypress so there's no need for a while() loop.
• Building with the default options and starting the resulting program from BASIC makes it return to BASIC so any output stays visible.

The latter implies that there's no need to change the source code of the program above in any way for the Apple II.

from cc65-chess.

If I go to an "old working" version of cc65, I get the same output as in MSVC and gcc from cc65.

That's the reason why it worked before.

I don't know what the standards say about any of this, but cc65 has changed its behaviour.

Even if the current cc65 behavior is covered by the standard it might still be that the cc65 team isn't aware of the change in behavior and considers it desirable to revert to the former behavior. I personally even assume that they see it this way.

My suggestion would be therefore to further reduce your program to the bare minimum showing the difference to GCC and open a cc65 issue (incl. the info which commit changed the behavior).

from cc65-chess.

Hi,
FYI, the regression has been fixed upstream in cc65/cc65#2398, in case you want to remove the workaround commit.

from cc65-chess.

@colinleroy Thanks for taking care of that! It's actually a nice show case for the usefulness of actually communicating with the cc65 team 😃

And that git bisect is a godsend 😄

from cc65-chess.