cpctelera / sdcc - order of variables in struct

[teopl]

I found a strange problem.

If I use the order of variables presented in the attached image (left side) the game reads h_px wrongly (some big value, didn't print it).

And if I use order from the right side, all is ok...

Now I know that order does matter in C, but I am not sure what am I doing wrong. How do you make order in structs? Do you use u16 at all?

[Arnaud]

Hello,
just tried with the following code (teoplEntityDirection is of course missing) :

Code: [Select]

typedef struct
{
u8 type;
u8 draw;

u16 x_px;
u8 y_px;

u16 x_px_prev;
u8 y_px_prev;

u8 w_px;
u8 h_px;

u8 index_tile;
u8* sprite;

u8 ticks;
u8 ticks_needed;

} STestStruct;

STestStruct gStruct;

void main(void) {

   gStruct.w_px = 10;
   gStruct.h_px = 11;

   printf("%d", gStruct.w_px);
   printf("%d", gStruct.h_px);

   // Loop forever
   while (1);
}
No problem, all works.

I haven't encounter structure order problem in my own developments.

Seems to come to the use of the structure in your functions.

[teopl]

Thanks Arnaud, I feared that it could be something else then the order...

The TeoplEntityDirection is an enum:

Code: [Select]

typedef enum {

    TEOPL_ENTITY_DIRECTION_NONE,

    TEOPL_ENTITY_DIRECTION_UP,
    TEOPL_ENTITY_DIRECTION_DOWN,
    TEOPL_ENTITY_DIRECTION_LEFT,
    TEOPL_ENTITY_DIRECTION_RIGHT

} TeoplEntityDirection;
but I guess it doesn't matter.

Also, one more thing - all works well in both cases when using wincpctelera from visual studio...

I must admit this is frustrating, because it looks like all other entity values are correct (exept the h_px) so it doesn't look like some memory overwriting which I already had once

[Arnaud]

Are the values in w_px and h_px OK just after the initialisation ?

If you want, you can put your project here, i'll take a look. Or send it me in PM.

[teopl]

Yeah, I think they should be. I will send you the project in PM.

Thanks again.

[teopl]

Just for the record, Arnaud found the problem:

I had empty Z80CCFLAGS in my build_config.mk file... 

After setting it to:

Code: [Select]

Z80CCFLAGS    := --max-allocs-per-node 8000 --opt-code-size
the issue is gone

[freemac]

You can cast struct into array of bytes, you can insert also declare an array of character just before like "HELLO IM HERE", and refound it using a hexadecimal software like HxD.

you can also initialize a global struct with values.

Code: [Select]

struct CALQUE_J1A {
   CALQUE marcher; // cyclique, avance, recule
   CALQUE haut;
};

// J1A.adresse : bank4_4000();
const struct CALQUE_J1A J1A= {
   .marcher={0,8,{{0,0},{0,0},{0,0}},BANK_4,MARCHE | MARCHER | RAPIDEMENT},
   .haut={9,0,{{0,0},{0,0},{0,0}},BANK_4,0},
};

(...)

bank0123();
LoadFile("J2.map", (char *)normDIR[PERSO_LIU_KANG]); // des sprites
LoadFile("J2.dir", (char *)mapping_direction_calque[PERSO_LIU_KANG]); // des directions
LoadFile("J2.pha", (char *)mapping_phase_calque[PERSO_LIU_KANG]); // des phases


[ronaldo]

Just for the record, Arnaud found the problem:
I had empty Z80CCFLAGS in my build_config.mk file... 
After setting it to:

Code: [Select]

Z80CCFLAGS    := --max-allocs-per-node 8000 --opt-code-sizethe issue is gone

That makes no sense at all. It seems more likely that there is an actual problem with some kind of undefined behaviour in your code, and different compilation options could be hidding the problem, not fixing it. Although a bug in the compiler is possible, it seems more plausible to me that what you describe is a bug in your code that either overwrites memory or fails to properly initialize variables.

In fact, the order at which you place elements in your structs defines their layout in memory. So, if you change order, you are effectively placing them in a different memory order and location. Also, if you change compilation options, you will produce a different code with different size and layout. All of this is probably hidding the problem, not fixing it.

I would rather have a deep look at your code. Most probably, the bug is still there.

[teopl]

@freemac Idea about packing struct in byte array sounds interesting... But it's not clear to me from the code you posted, maybe you could post some working example?

@ronaldo yes, the issue sounded very much like a memory problem since I had strange problems like: it showed and appeared randomly when code is changed in places not even connected to this module.

but since then, I changed and updated the code a lot and I didn't have issue yet so I will presume it's gone

anyway, one thing to say: I also had problems (hanging, crashing, not able to play some songs ...) with arkos2 player (provided here on the forum) and I switched to latest cpctelera and arkos1 so since then all is good, so I will continue like that.

[Targhan]

@teopl If you have time and if you deem it useful, could you switch to the latest version of Arkos Tracker 2? There is a special source converter which converts the player/music source directly to the SDCC syntax. There is a tutorial here about that.

[teopl]

@Targhan Hi, thanks for tutorial and for Arkos also! I will definitely try it, just probably not very very soon because of time limitation and we will not go into music yet (maybe in month or so).
For now build system and code is reverted to use Arkos1/cpctelera and when we make enough progress in game, I will for sure first check if all is working before starting learning/composing for Arkos2...

That brings me to one question: Is it or will it be possible to import Arkos1 song to Arkos2? (and are there any problems with that if yes)

Also, I am almost totally unfamiliar with differences between Arkos1 and Arkos2 since I do the code and this is my first cpc project, so a lot to learn...

[Targhan]

That brings me to one question: Is it or will it be possible to import Arkos1 song to Arkos2? (and are there any problems with that if yes)

Yes absolutely, importing AT1 songs works very fine (your song may need some tweaking for some very special hardware sound, but that's it).

Also, I am almost totally unfamiliar with differences between Arkos1 and Arkos2 since I do the code and this is my first cpc project, so a lot to learn...

To sum up, AT2 is much better than AT1 . Anyone who's used to AT1 can switch to AT2 almost seamlessly.

[teopl]

The time has come to try once more to play arkos2 song with cpctelera...

I have made a short program which should:

- convert any "song_XXX.aks" and generate configuration using command line Arkos2 conversion tool- include all (player, configuration and songs) into one .rasm file and convert it to .s which can be used by sdcc/cpctelera (followed inctruction from Targhan)

Use like this:

Code: [Select]

./make_all_with_colors.sh
./run.shProblem:

- it doesn't work when I uncomment line:

Code: [Select]

//__asm__("call _PLY_AKG_PLAY"); Now, sorry if I made some fundamental error, I really don't quite get how this works and I didn't have any more time to investigate...
(because I aready have all working with Arkos1 cpctelera player)

So, any idea what I am doing wrong here?

[teopl]

This is main file:

Code: [Select]

#include <cpctelera.h>

extern void* SONG_EXCELLENCE_START;

static const u8 palette[16] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15 };
static u8 _tick;

// ; Initializes the player.
// ; IN:    HL = music address.
// ;         A = subsong index(>= 0).
void sound_song_init(void* song) __z88dk_callee __naked {

    __asm;

        ; get params from stack
        pop     af; ret address first because __z88dk_callee convention
        pop     hl
        push    af; ret address last  because __z88dk_callee convention
        ld      a, #0

        ; ##########################################

        call _PLY_AKG_INIT

        ; ##########################################

        ret

    __endasm;
}

static void interrupt_handler(void) {

    _tick++;

    if (_tick == 6) {

        _tick = 0;

        //__asm__("call _PLY_AKG_PLAY");
    }
}

void interrupt_init() {

    _tick = 0;

    // ---

    cpct_waitVSYNC();
    __asm__("halt");
    __asm__("halt");
    cpct_waitVSYNC();

    cpct_setInterruptHandler(interrupt_handler);
}

void sound_init() {
    sound_song_init(SONG_EXCELLENCE_START);
}

void main() {

    u8 index = 0;

    // ---

    cpct_disableFirmware(); // not to interfere with code

    cpct_setVideoMode(0);
    cpct_clearScreen_f64(0);
    cpct_fw2hw(palette, 16);
    cpct_setPalette(palette, 16);

    sound_init();
    interrupt_init();

    while (1) {
        cpct_setBorder(palette[index++ % 16]);
        cpct_clearScreen_f64(0);
        cpct_waitVSYNC();
    }
}

[ronaldo]

Hi @teopl ,

   One basic recommendation you should always take into account is to thoroughly read all documentation from the functions you use. CPCtelera functions are no magic bullets that can work on everything you want. You are the programmer and you must now how everything works: that's your responsability. The library just helps you in saving time not having to implement functions by yourself, but it does not prevent you from having to know the details.

   I say this because you are using cpct_clearScreen_f64(0) in your main loop clearly without having read documentation. It basically is a macro that calls cpct_memset_f64, which clearly states this in the documentation:

C Definition

void cpct_memset_f64 (void* array, u16 value, u16 size);

Warning

• This function disables interrupts while operating and uses the stack pointer.  Take it into account when you require interrupts or the stack pointer.  When in doubt, use cpct_memset instead.

    Basically, it disables interrupts and it takes more than 1/50s, so it will ruin any sincronization you are trying to accomplish with your interrupts.

    Your code and project may have other problems, but this is clearly an important flaw. Please, review it and we'll try to help you with other potential problems.

[teopl]

@ronaldo You are right, that is one issue. But in  my defense I did read about that in the doc, I just didn't think 
(I just quickly copy pasted that while trying to find what is the issue, at first I tried with totally empty while loop and is also didn't work)

So the "real" issue was quite simple: I didn't send address of a song pointer as a parameter to init.

So,

this is correct:
sound_song_init(&SONG_EXCELLENCE_START);
this was not:
sound_song_init(SONG_EXCELLENCE_START);

And now I am very happy with the results, since I have automated way for arkos2 playback using only sources and tools provided in Arkos2 bundle

I am attaching fixed source.

Thanks @Targhan for the instructions and the tools!

[ronaldo]

this is correct:
sound_song_init(&SONG_EXCELLENCE_START);
this was not:
sound_song_init(SONG_EXCELLENCE_START);

Well, unless you have a rare definition of SONG_EXCELLENCE_START, this does not make any sense. SONG_EXCELLENCE_START is declared as extern void*, so it should already be a memory address, and seems absurd to pass a double pointer to song init. But if it works, that means your definition of SONG_EXCELLENCE_START, whereever it is, does not properly match that "extern void*" declaration. If taking its address does make sense, then you should have defined SONG_EXCELLENCE_START as the first item in the song array, and not the address where the song array starts.

For this kind of problems, the best you can do is debugging directly in the emulator and checking the values of the parameters. That way you should immediately notice that the value being passed is not the address where the song starts. Spending time debugging low level is greatly beneficial to better understand how things work. Helps a lot to improve coding skills.

[teopl]

Hi ronaldo,

First, you are right about the debugging from emulator, I just didn't have the courage but I will get used to it as soon as possible since that could save me some time with things like this, memory overwriting, etc...

And for the (solved) issue:

The definition of the SONG_EXCELLENCE_START is in the generated assembler code and it looks like this:

Code: [Select]

_SONG_WELCOME_START:: SONG_WELCOME_START: .db 65
    .db 84
    .db 50
    .db 48
    .dw SONG_WELCOME_ARPEGGIOTABLE
    .dw SONG_WELCOME_PITCHTABLE
    .dw SONG_WELCOME_INSTRUMENTTABLE
    .dw SONG_WELCOME_EFFECTBLOCKTABLE
    .dw SONG_WELCOME_SUBSONG0_START
If was not globally visible so I added a '_' before and ':' to the end automatically from a script to be able to declare extern void* in my .c program.

My understanding may be wrong, but I thought that labels in asm are in fact a memory address so I did declared it like a pointer.

The init function which takes this song pointer (or whatever) is defined in asm like this:

Code: [Select]

;Initializes the song. MUST be called before actually playing the song.
;IN:    HL = Address of the song.
;       A = Index of the subsong to play (>=0).
PLY_LW_InitDisarkGenerateExternalLabel:
PLY_LW_Init:
and my .c program is calling this init function like this:

Code: [Select]

void sound_song_init(void* song) __z88dk_callee __naked {

    __asm;

        ; get params from stack
        pop     iy; ret address first because __z88dk_callee convention
        pop     hl
        push    iy; ret address last  because __z88dk_callee convention
        ld      a, #0

        ; ##########################################

        call _PLY_AKG_INIT

        ; ##########################################

        ret

    __endasm;
}
So I am not sure what is the issue, but it does work when I do init like this:

Code: [Select]

sound_song_init(&SONG_WELCOME_START);
no matter what is the extern declaration:

this:

Code: [Select]

extern u16 SONG_WELCOME_START;
or this:

Code: [Select]

extern void* SONG_WELCOME_START;

[Targhan]

The song global declaration missing was an oversight, it has been added in the next version of AT2.

[teopl]

Great, and if it's not too much or breaking something, maybe song label can be generated in the same way for all players.

I mean that, when I use AKG the song is ending with "_START" and with LightWeight, it's ending with "_SONGSTART".

[Targhan]

Well, too late for the next release, I'll see about that next time.