The Dink Network

Hello everyone,
Just a brief introduction to me... I'm Keiran the new GNU Freedink maintainer at the GNU Project.
Apologies that the project has been stagnant for quite some time but I'm keen to make some progress.
If anyone has any questions, bugs or other matters please feel free to let me know

Hey, nice timing!

We were just talking in a newly released DMOD thread about some BIG Freedink bug in 109.6, on 64 bit linux releases. http://www.dinknetwork.com/forum.cgi?MID=207767&Posts=60

Long story short, ghostknight discovered that the code uses "long" instead of "int" in some places and this leads to some things breaking when passing function parameters.

To start with, check line 50 in dinkc.cpp

/* store current procedure arguments expanded values of type 'int' (see get_parms) */
static long nlist[10];

I'm not at my freedink computer right now, but I remember there being other instances of misusing long instead of int too. Check out ghostknight's post in that thread, and I'll get back to you later with whatever else I can find.

I'm pretty sure that there shouldn't be *any* longs at all, since I don't think dink was developed with 64bit variables in mind. Most likely, I imagine that any use of "long" is some old legacy code where Seth wanted to make sure 32bit values were used instead of 16bit values...

There is one more REALLY annoying thing that we'd all be really happy if someone fixed it, but it's not exactly a bug or related to any of this, just a quality of life thing: When Freedink switched to SDL2, there was a change in the SDL code that causes "DEBUG: Surface doesn't have a colorkey" to be spammed to the debug console stream. This effectively floods the debug log and the onscreen debug info in the game. Beuc talked about this in this issue: https://github.com/libsdl-org/SDL/issues/2909

I don't know if this can be fixed by modifying calls to SDL_GetColorKey in some way, or it needs a custom SDL2 build where the debug message change gets reverted...

I have spotted some following bugs from Linux side on my end:
1. FreeDink fails to compile on recent SDL2 since 2.0.10 the extension SDL_HINT_ANDROID_SEPARATE_MOUSE_AND_TOUCH has been replaced with SDL_HINT_MOUSE_TOUCH_EVENTS and the software still has the former in the input.cpp
2. Autoconf fails when running bootstrap.sh complaining over the AC_PREREQ being on 2.61, suggested 2.64 as a minimum in configure.ac
3. Compiling fails with this following error:

gfx_fonts.cpp: In function ‘void setup_font(TTF_Font*)’:
gfx_fonts.cpp:296:44: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
  296 |   char *familyname = TTF_FontFaceFamilyName(font);
      |                      ~~~~~~~~~~~~~~~~~~~~~~^~~~~~
      |                                            |
      |                                            const char*
gfx_fonts.cpp:299:42: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
  299 |   char *stylename = TTF_FontFaceStyleName(font);
      |                     ~~~~~~~~~~~~~~~~~~~~~^~~~~~
      |                                          |
      |                                          const char*
gfx_fonts.cpp: In function ‘void setup_font(TTF_Font*)’:
gfx_fonts.cpp:296:44: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
  296 |   char *familyname = TTF_FontFaceFamilyName(font);
      |                      ~~~~~~~~~~~~~~~~~~~~~~^~~~~~
      |                                            |
      |                                            const char*
gfx_fonts.cpp:299:42: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
  299 |   char *stylename = TTF_FontFaceStyleName(font);
      |                     ~~~~~~~~~~~~~~~~~~~~~^~~~~~
      |                                          |
      |                                          const char*

4. FreeDink crashes with SIGSEGV (Segmentation fault) when a specific sound plays like hitting an object or the fire plays the crackling sound, it does not happen when playing with no sound.

This is on ArchLinux.

As for questions i have some of em:
- Will FreeDink be expanded in terms of features compared to the original? Stuff like image transparency support, setting up the opacity for the sprite via script etc.
- Will it have a dedicated main menu that can be operated separately to allow the user set the controls up, resolution etc. or even bind custom commands which some mods have into the controller?
- Any chance to replace the current compiler with newer ones such as CMake or Meson?

Gokussj6: Will FreeDink be expanded in terms of features compared to the original?

Can I add to this.. I would like to say, if any new features are implemented as such, it is probably desirable it is implemented in such a way that it does not break compatibility with earlier dmods. Also, a command accessible by dmod authors in DinkC that returns the current FreeDink version would be a great addition.

Also. Bad Robj is double posting again instead of editing.

Anyway.

If you haven't seen it yet, there was a problem with the latest 109.6 build of FreeDink on Linux, that was causing my latest Dmod, "Charlie's Legacy" to not play correctly. Specifically, this bug was picked up during a tile puzzle I scripted. The tile puzzle controls did not function at all, the game lagged on screen entry and the variables and scripts maxed out. I originally thought it was just the arguments being passed to the procedures were getting messed up by FreeDink, but some people discovered there were other issues (which I don't understand - I'm not a programmer ). Also, the procedure argument thing confused me because I recently ran a test in a new dmod, and now, they seem to be working correctly in that, so I honestly don't know what ithe actual issue is(Although it seems other people do). You may want to take a look at that discussion yourself. Link to where ghost knight talks about the actual fixes or something:
Post link

@Robj, I literally talked about just that and linked the thread in my post above.

Edit: Although to be fair I didn't link the exact post...

Alright @kjh, so here is my BIG comprehensive list of misused longs in freedink, i think it covers everything:

----------- Critical misuse of long ----
As discovered by ghostknight, these are gamebreaking in certain scenarios and need to be int.

// dinkc.cpp, line 51:
    /* store current procedure arguments expanded values of type 'int' (see get_parms) */
    static long nlist[10];
    
// dinkc.cpp, line 1748: 
// here there is a size mismatch between declaration and memset use
      memset(nlist, 0, 10 * sizeof (int));
      
// dinkc.cpp, line 603
// this function stores its return value in nlist, it should also be int
    long decipher(char* variable, int script)
    
// dinkc.h, line 116
    extern long decipher(char* variable, int script);

----------- Probably has no impact ----

//dinkc.cpp, line 72
//in the call_back struct, the min/max variables are long when 
//they should be int, they are only used in combination with other ints

    struct call_back
    {
      int owner;  // script ID
      /*bool*/int active;
      int type;
      char name[20];
      int offset;
      long min, max;
      int lifespan;
      unsigned long timer;
    };

----------- Timing variables, need to be Uint32, possible issues? ----
All of these instances are linked to variables that are used for timing / game tick management. I *THINK* they are supposed to be Uint32 and NOT Int32, it probably doesn't matter if they are 64 bit, but it can cause issues if compiled for x86 with 32 bit longs i think?...

I say they should be Uint32, because they are used in relation to variables which are indeed declared as Uint32

IMPORTANT NOTE: Robj, please correct me if timer overflow from 2147483647 to -2147483648 is ever used in magic DMOD trickery. I expect it shouldn't though, and letting timers run from 0 to 4294967296 should be fine?...

//game_engine.h, line 83
    extern Uint32 thisTickCount;
    extern Uint32 lastTickCount;

Variable cycle_clock:

// game_engine.cpp, line 91
    unsigned long cycle_clock = 0;

// game_engine.h, line 78
    extern unsigned long cycle_clock;

// it interacts with thisTickCount in...

// dinkc_bindings.cpp, line 1000
// this would cause issues if compiled for x86
    cycle_clock = thisTickCount+1000;
    
// freedink.cpp, line 436
// again, possible issues if compiled for x86 i think
    if (thisTickCount > cycle_clock)

Variable mold:

[code]
// update_frame.cpp, line 64
    static unsigned long mold;
    
    
// mold interacts with thisTickCount in...

// update_frame.cpp, lines 231, 232
    if (thisTickCount > mold+100) {
        mold = thisTickCount;

Struct sp in live_sprite.h

//live_sprite.h, line 45, 51, 96
    unsigned long delay;
    ...
    unsigned long wait;
    ...
    unsigned long notouch_timer;
    ...

// all of these interact with thisTickCount in MANY places, therefore they
// should probably also be of the same type, Uint32

----------- Used in file access, needs to be AT LEAST 32 bit ---
None of the dink files can ever reach sizes ranging in the GB, so 32 bits are more than enough for file access.

Variables of the struct refinfo, these all are used in file access, need to be at least Int32, but 64 bit not ever needed

//dinkc.h, line 58, 59
    long location;
    long current; // current offset
    ...

//dinkc.h, line 64
    long end; // size of the text, == strlen(rbuf[i])
    ...

These variables seem to be used in fseek, so technically it is not wrong for them to be "long", however, no map.dat file will ever be big enough to require long, so i think they should be changed for consistency anyway

//editor_screen.cpp, lines 97 and 211
    long holdme,lsize;

These variables are used to refer to some file offsets in the dir.ff files. As far as i am aware, dir.ff files use 32 bit when storing offsets internally, so these should be Int32 as well...

// fastfile.cpp, line53
    struct FF_Entry
    {
      unsigned long off;
      char name[13];
    };

// fastfile.cpp, line 60  
    struct FF_Handle
    {
      int alive;
      unsigned long pos, off, len;
    };

// fastfile.cpp, line 88
    FastFileInit(char *filename, int max_handles)
    {
      unsigned long count = 0;
  
// fastfile.cpp, line 222
    FastFileOpen(char *name)
    {
      struct FF_Handle *i;
      long fCount;
      long hCount;

// fastfile.cpp, line 226
    for (fCount = 0; fCount < (long)g_numEntries - 1; fCount++)
  
// fastfile.cpp, line 230
    for (hCount = 0; hCount < (long)g_numHandles; hCount++)

// fastfile.cpp, line 241
    unsigned long next_off = g_Entries[fCount + 1].off;

its a damn good thing you didn't have issues with signing up, too.

>Most likely, I imagine that any use of "long" is some old legacy code where Seth wanted to make sure 32bit values were used instead of 16bit values

Good point, and thanks for the write-up. Looking at the original Dink source, it appears that Beuc copied and pasted the DinkC parser verbatim into Freedink. In the case of those other data types and perhaps anything else we haven't stumbled upon yet that's broken on x64, it might be worth investigating the RTDink source to see how things are defined, as surprise surprise, nlist[10] is an int32 there.

it might be worth investigating the RTDink source to see how things are defined, as surprise surprise, nlist[10] is an int32 there.

There is still a bug, though, in the original source since in line 4013 it does

memset(g_nlist, 0, 10 * sizeof(int));

On a system where int is only 16 bit then int32 would probably would have been typedef'ed as long. In that case the memset would still only delete half the array.

Mr Maintainer-san, are you still around?... Hope we didn't scare you off with our long posts.

Hopefully he's just busy working on these fixes

So far, the new maintainer seems to have mustered up the effort to add his name to the main page of the Freedink site but otherwise hasn't made a single git commit. Time does move slowly in the Dink world, but considering that the most urgent of the fixes is a one-liner that would take a few minutes to alter at most given that ebilv and ghostknight have provided the replacement line, I can't say I have high hopes at this point.

Also gokussj, you can sort that by running ./configure CXXFLAGS=-fpermissive before running make.

"Time does move slowly in the Dink world, but considering that the most urgent of the fixes is a one-liner that would take a few minutes to alter at most given that ebilv and ghostknight have provided the replacement line, I can't say I have high hopes at this point."

I sent him an email, to politely ask if he's seen these responses, and also to clarify that, rather than trying to pester him, it is more so rather excitement that there is a new maintainer, and also hope that these errors can be ironed out.

Did the new maintainer disappear into thin air?

yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyep

Nevermind, he's online now.
EDIT: And didn't post a reply....

Hey @drone1400, I believe that I've amended all of the issues that you've described.
I've created this as branch 109.7 in the freedink git repository. You can check this out using:

git clone https://git.savannah.gnu.org/git/freedink.git
git branch -a

Hi @RangerLord,
I apologise for the delay. Unfortunately life has gotten in the way. I believe I've completed some of these changes as detailed in a response.
Note that the easiest way is to contact me is via email.

Keiran

Hey @keiran! That's great news! Btw, in your absence, another user has made a bunch of fixes that you might want to check out, here is his freedink git: https://codeberg.org/crts/freedink

I've just added these changes to the same branch.

Ah, nice to see FreeDink is finally on the way of getting updated. Too bad I haven't been doing anything Dink related for a few weeks now.

you just did.

Okay, other then reading and writing in the forums.

logging in

Someone is being very picky. (Don't ask why I picked this emoticon, I'm not sure myself.)

I've just added these changes to the same branch.

This is not OK. I have made several improvements/bugfixes to appr. a dozen files and added a copyright notice to the headers of the affected files in order to indicate that I am the author of those changes. The new maintainer removed every trace of that!

What's worse, he even took credit for an entirely new script in contrib/log-format.sh which I wrote from scratch! He deleted me and added his name instead as author.

Here is what he put in his repository. It is a verbatim copy of my script except for the copyright notice.

After hearing that freedink has a new maintainer but no work has been done on it, my initial thought was that the new maintainer just wanted to add freedink to his CV as reference. The recent actions seem to confirm that.

Some of those changes I added *might* seem "unspectacular", but it still took some good time to investigate those bugs/improvements. I spent a few afternoons working on that.

I was going to do some further work on improving/fixing freedink but not under these circumstances. While I only do this in my spare time for fun, I have always believed that credit should be given where credit is due. What really bugs me, though, is that someone else takes credit for my work.

This is not OK!

Agreed. This is a clear violation of the GNU GPL, and very disappointing/alarming to see coming from a GNU Project representative. I think at this point we're better off just maintaining our own fork(s).

Agreed. This is a clear violation of the GNU GPL, and very disappointing/alarming to see coming from a GNU Project representative.

Thanks for taking the time to verify/confirm my points. Please make also sure to clone his git repository, as evidence

git clone https://git.savannah.gnu.org/git/freedink.git
cd freedink
git branch -a

Keiran dude, wtf are you doing?

@ghostknight I suppose you should contact the GNU Savannah folks about this if you haven't already.

@ghostknight, I apologise for any distress caused due to my lack of care. I have removed the branch in question and would invite you to contribute directly to the Savannah repository and can set up access if you wish to do this.
GNU Project policy states that contributions do get logged and this is something I missed.
I only contribute to the GNU Project in my spare time and without any further support with programming, documentation writing or finding bugs, I at some point will likely make mistakes.
I invite anyone who wishes to make changes to the various packages to ping me an email on kjh (at) gnu.org and I will make sure I get things sorted.
Keiran

doesn't really touch upon why you would decide to actively remove someone's credit and replace it with your own in the code. that's not a "lack of care" or "will likely make mistakes because i do it in my spare time". that's "hey, i'm gonna consciously pretend i wrote this and that the other guy doesn't exist"

Again I can only apologise for the issues I've caused. I have spoken with my contact at the GNU Project to ensure this doesn't happen again.

Keiran

so, someone else did that now?