search

sezero / mikmod

Mikmod Sound System (mirror of git repo at https://sf.net/projects/mikmod/)
http://mikmod.sourceforge.net/
72 stars 22 forks source link

add n64 driver #3

Closed vieux closed 5 years ago

sezero commented 5 years ago

Thank you!

  1. Nitpicking: N64_Init() should return int, not BOOL. (can fix it after myself I merge.)

  2. About the workaround added to mmio.c for LONG_MAX: I am not comfortable with it. Does your SDK not have a limits.h (your CFLAGS in your makefile doesn't have -DHAVE_LIMITS_H)? Or does it not define LONG_MAX (how??)

vieux commented 5 years ago

Sure, let me fix 1. as soon as I'm back on a computer. Regarding 2. I'll investigate.

sezero commented 5 years ago

FWIW, I just made simplifications: see attached patch. It builds for me using the old precompiled windows-hosted toolchain from 2009 at https://dragonminded.com/n64dev/

simplified_patch.txt

vieux commented 5 years ago

I pushed a few changed based on your patch, thanks. one question though.

I used the libdragon's examples as template for the makefile, I'm not sure having $(N64_INST)/bin in your PATH is common practice with libdragon, could we do with those changes ?

sezero commented 5 years ago

OK, merged it as it is. Thanks!

vieux commented 5 years ago

@sezero oh thanks for the quick merge, just to be sure, the current state of the commit is that you do cd n64 && make -f Makefile.n64 is that consistent with the other Makefiles ?

sezero commented 5 years ago

cd n64 && make -f Makefile.n64 is that consistent with the other Makefiles ?

Yes.

sezero commented 5 years ago

Just added -DHAVE_SNPRINTF to the makefile, you may want that. (commit 600b62f9e993570c7de26d642eac36e3a91bf139)

sezero commented 5 years ago

Pushed two or three further commits - you may want to pull.

sezero commented 5 years ago

Pulled from your branch and pushed a minor simplification to N64_Update(). Please make sure things are OK, so that I can consider this as stable.

vieux commented 5 years ago

Cool thanks, I was about to send a PR for the can write. I'll make one last test and report back here

On Fri, Sep 27, 2019, 10:17 AM Ozkan Sezer notifications@github.com wrote:

Pulled from your branch and pushed a minor simplification to N64_Update(). Please make sure things are OK, so that I can consider this as stable.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sezero/mikmod/pull/3?email_source=notifications&email_token=AAH4CR6I7L5WH6N7457VPNTQLY5Y3A5CNFSM4IY5SOP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7ZRTQA#issuecomment-536025536, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH4CR23DZGTFPLA3DMFOA3QLY5Y3ANCNFSM4IY5SOPQ .

vieux commented 5 years ago

@sezero unrelated to the last changes, but I do see a warning during compilation:

../loaders/load_669.c: In function 'S69_Load':
../loaders/load_669.c:297:4: warning: 'strncpy' output may be truncated copying 36 bytes from a string of length 107 [-Wstringop-truncation]
  297 |    strncpy(of.comment,mh->message,36);
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

do you see the same on other platforms ?

sezero commented 5 years ago

do you see the same on other platforms ?

Yes, and ignoring it.

vieux commented 5 years ago

ok 👍 I was able to compile my game with the latest commit and it seems to work great

sezero commented 5 years ago

Great! I consider the n64 addition as stable then. Thanks.