tursilion / libti99

GCC Library for the TI-99/4A
28 stars 10 forks source link

[bug?] gotoxy macro may produce impredictable results #19

Closed Fabrizio-Caruso closed 2 years ago

Fabrizio-Caruso commented 2 years ago

In https://github.com/tursilion/libti99/blob/main/conio.h#L77 I see a macro with two commands with no do{...}while(0) to protect it from being misinterpreted. This might explain why I get totally wrong results when I use it to display text.

For example

if(...)
   gotoxy(x,y)

is wrongly interpreted as

if(...)
{
    conio_x = (x); 
}
conio_y = (y)

And you need do, while(0) and not just braces, because it would fail with a syntax error in cases such as

if(...)
    gotoxy(x,y);
else
    ...

So a solution would be to do:

#define gotoxy(x,y) \
do \
{ \
    conio_x = (x); \
    conio_y = (y); \
} while(0)

In general I would do this on all macros, even the ones with one single command to be safe just in case I replace that command with a macro one day.

tursilion commented 2 years ago

Ahhh, good call. Submit a pull request and I'll merge it.

tursilion commented 2 years ago

Yeah, to do a merge request, you have to create a fork, then do the fix in your fork. Once it's tested, you can then request a pull request back into the original.

Fabrizio-Caruso commented 2 years ago

I realized I did not create my own fork. I am retrying with the PR.

Fabrizio-Caruso commented 2 years ago

I have made a tentative pull request: https://github.com/tursilion/libti99/pull/20

Fabrizio-Caruso commented 2 years ago

By the way the reason why I started looking in conio.h is that I got (and still get after my "fix"): image

I will try to reproduce it with a minimal example. If it is a real bug, I will open a different issue.

tursilion commented 2 years ago

If it is a real bug, I will open a different issue.

Or you could just email me so I don't have to keep opening a web browser. I don't live on the web, it's a terrible, sticky place. ;)

Just because github supports "issues" doesn't mean we have to use them. Github supports lots of bad things. ;)

I'll need some kind of source from you to help, yes. Screenshots tell me nothing.

Fabrizio-Caruso commented 2 years ago

@tursilion, Of course the picture is not enough. I am trying to reproduce it. I would not ask you to fix things asap or any time soon. I only wanted to report a problem and provide a solution. I can send bug reports to you by mail. I won't open issues here if you don't want to. Sorry.

Fabrizio-Caruso commented 2 years ago

Thanks for the merge!

tursilion commented 2 years ago

Issue appears to be failure to call vdp_setgraphics() to set up the screen size variables, for anyone reading this with a similar issue.