AGS 3.6.0.35 (Linux and OSX crash)

Started by Dualnames, Thu 13/10/2022 17:23:20

Previous topic - Next topic

Dualnames

So, I'm trying Strangeland with 3.6, everything seems to work fine and perfectly, except on Linux and MacOS, it crashes whenever this function runs:

Code: AGS
bigSave=DynamicSprite.CreateFromFile(String.Format("$SAVEGAMEDIR$/saveHD%d.bmp",getSlot));

Well, it doesn't crash there per se, it crashes after it, if you try to access bigSave or whichever dynamic sprite you assigned with CreateFromFile. So to summarize, only on LINUX AND ON MACOS, if you do "CreateFromFile" with a Dynamic Sprite and then try to access it, it goes kablooie.

(And yes the file in question exists. It returns a null pointer. I'm wondering if this an omission on my end and the pathing needs to be written differently depending on OS?)
Worked on Strangeland, Primordia, Hob's Barrow, The Cat Lady, Mage's Initiation, Until I Have You, Downfall, Hunie Pop, and every game in the Wadjet Eye Games catalogue (porting)

Crimson Wizard

#1
First of all, to clarify, this "crash" is not a program crash, but engine reports a "null pointer" error in script because bigSave was not assigned? So, the issue is not really that it crashes (it errors logically), but that it cannot create a sprite from the file?

Where the file is located, and what is the save game dir? The engine log should print the game paths, including save dir, could you double check that?

How is the file supposed to appear in the save game dir in the first place?

eri0o

So it doesn't crash, opening that file gives a null pointer? If that file exists there, and you not write to it just before, but reads later, does it still gives a null pointer?

eri0o

#3
My guess, disable disk cache on writing in Windows, and see if you reproduce the behavior you are seeing in macOS and Linux.

From what I remember, in Windows the file writing is kinda of assynchronous, it goes to cache and then to the disk, so if you read before it's effectively on disk, Windows reads from the cache transparently. I thought that macOS and Linux engine would block the writing, so it would not fail to read because it would wait the write operation finish. So I don't know exactly why it's happening, but my first guess was this actual difference in OS behavior. In fact the blocking behavior was why in the past I suggested assynchronous save files/streams.

My suggestion would be simply handling the nullpointer case.

morganw

Just to check, have you just printed the slot number to check that you are actually reading the correct file?
If everything looks OK then you can probably work out what is going on by running the game through strace.

Dualnames

it is reading the correct file, for a second even the sprite is properly assigned to the UI button, and then it goes back to no graphic and then when the script reaccesses it it returns null. Which is absolutely weird.
Worked on Strangeland, Primordia, Hob's Barrow, The Cat Lady, Mage's Initiation, Until I Have You, Downfall, Hunie Pop, and every game in the Wadjet Eye Games catalogue (porting)

Crimson Wizard

#6
Quote from: Dualnames on Sun 16/10/2022 10:46:17it is reading the correct file, for a second even the sprite is properly assigned to the UI button, and then it goes back to no graphic and then when the script reaccesses it it returns null. Which is absolutely weird.

If the bigSave variable is assigned properly first, then this means that the CreateFromFile call succeeded, at least  the first call.

Do you have bigSave variable reassigned anywhere else in script? How often, under which conditions do you have this line called? In which context do you have this line in your script?

E.g. do you call it in a loop? or repeatedly over time perhaps?

Dualnames

So, here is the funny part.

Code: AGS
bigSave=DynamicSprite.CreateFromFile(String.Format("$SAVEGAMEDIR$/saveHD%d.bmp",getSlot));
Wait(10);
bigSave.Resize(); //Resize is not what matters


This crashes. No matter what. The wait doesn't exist on my end. I am quite sure I don't do anything else after creating the sprite to cause this issue per se. This line gets called whenever you save the game. It runs once during that time.
Worked on Strangeland, Primordia, Hob's Barrow, The Cat Lady, Mage's Initiation, Until I Have You, Downfall, Hunie Pop, and every game in the Wadjet Eye Games catalogue (porting)

Crimson Wizard

#8
Quote from: Dualnames on Sun 16/10/2022 23:48:46This crashes. No matter what.

Do I guess correctly that the crash is at bigSave.Resize() because of null pointer, and you get the "null pointer" script error on that line?

Have you tried testing for CreateFromFile return value right after it is called?

Testing if sprite was loaded correctly is a right approach anyway. CreateFromFile returning "null" is a valid behavior, this may happen in case the file does not exist, or could not be read for any reason (corrupted etc). Game script should not rely on this always succeeding.

Quote from: Dualnames on Sun 16/10/2022 23:48:46The wait doesn't exist on my end.

Could you please clarify what does that mean, what is "your end"?

Khris

You say the file exists but have you checked the result of Display("$SAVEGAMEDIR$")?
If this token isn't replaced by the proper path on non-Windows systems, the path is going to be invalid and the DynamicSprite pointer will point to null.

Luckily this is really easy to debug.

Crimson Wizard

#10
Quote from: Khris on Mon 17/10/2022 01:08:45You say the file exists but have you checked the result of Display("$SAVEGAMEDIR$")?

These tokens are not resolved to a path in Display command, only in File commands (and ListBox's FillDir).
There is an open feature suggestion for giving a function that returns these resolved values, but it has not been implemented yet.

You may find out what is the paths are by reading the engine logs, or running the engine or game executable with "--tell-filepath" command arg.
this page mentions how to do this: https://github.com/adventuregamestudio/ags/blob/master/OPTIONS.md


Dualnames

So prior to createfromfile, there is a check to check if the file exists.
So this is some sort of reading error. Investigating further.
Worked on Strangeland, Primordia, Hob's Barrow, The Cat Lady, Mage's Initiation, Until I Have You, Downfall, Hunie Pop, and every game in the Wadjet Eye Games catalogue (porting)

Dualnames

#12


So the issue from what i can tell is besides being able to access the savegamesdir (I have cloud saves so it was trying to access the directories and files)
Is that it's unable to save into the $SAVEGAMEDIR$.

When there is a space on MAC, the Application Support part should be Application\ Support, otherwise it won't be able to read it.
So when the files are not there, it doesn't crash, but when they are it crashes being unable to read from the directory.

However this is the difference here, is that,
Code: AGS
 if (File.Exists("$SAVEGAMEDIR$/saveHD0.bmp")) 

Reads properly and returns true. But
Code: AGS
 DynamicSprite.CreateFromFile("$SAVEGAMEDIR$/saveHD0.bmp");

Doesn't.


Code: AGS
if (File.Exists(String.Format("$SAVEGAMEDIR$/saveHD%d.bmp",getSlot))&& doubleclickload)
    {
            
      if (bigSave!=null) bigSave.Delete();
      bigSave=DynamicSprite.CreateFromFile(String.Format("$SAVEGAMEDIR$/saveHD%d.bmp",getSlot));
      
      bigSave.Resize(gwidth, gheight);
}

I'm wondering if the Delete is the outlier, so gonna test this by removing the delete part!
EDIT: It's not.
Worked on Strangeland, Primordia, Hob's Barrow, The Cat Lady, Mage's Initiation, Until I Have You, Downfall, Hunie Pop, and every game in the Wadjet Eye Games catalogue (porting)

Dualnames

Further investigation:

This is 100% replicatable, with a new project, same code.
Put this on room_load or room_after fade in.
Create a dynamic sprite variable naming it "spriteFile"

Code: AGS
bool isnull=false;

function room_Load()
{  
    if (spriteFile==null && File.Exists(String.Format("$SAVEGAMEDIR$/saveHD%d.bmp",0)))
    {
        spriteFile=DynamicSprite.CreateFromFile(String.Format("$SAVEGAMEDIR$/saveHD%d.bmp",0));
        bool getexists=File.Exists(String.Format("$SAVEGAMEDIR$/saveHD%d.bmp",0));
        int a=0;
        while (spriteFile==null && a<100)
       {
            spriteFile=DynamicSprite.CreateFromFile(String.Format("$SAVEGAMEDIR$/saveHD%d.bmp",0));      
           a+=1;
       }
       if (spriteFile==null)isnull=true;    
       else isnull=false;

       AbortGame(String.Format("File exists: %d %d", getexists, isnull));
  }
}

1) Run the game once. It should return "File exists: 0 1"
2) Create a bmp called saveHD0.bmp in your savegames directory of this project.
3) Start the game again it will return "File exists: 1 1" but only on LINUX and OSX. On Windows it will return "File Exists: 1 0"

Either, I'm absolutely stupid, or this shouldn't be the case.
Worked on Strangeland, Primordia, Hob's Barrow, The Cat Lady, Mage's Initiation, Until I Have You, Downfall, Hunie Pop, and every game in the Wadjet Eye Games catalogue (porting)

Crimson Wizard

#14
So, it's a question of whether CreateFromFile is working properly on Linux and Mac; whether it a) works at all and whether it b) works with particular paths.
I guess the easier test could be to use File.Open.

Speaking of system differences, besides from being unix systems, Linux and Mac have case-sensitive filepaths, so that might also affect file reading.



Quote from: Dualnames on Mon 17/10/2022 05:59:52So the issue from what i can tell is besides being able to access the savegamesdir (I have cloud saves so it was trying to access the directories and files)
Is that it's unable to save into the $SAVEGAMEDIR$.

When there is a space on MAC, the Application Support part should be Application\ Support, otherwise it won't be able to read it.
So when the files are not there, it doesn't crash, but when they are it crashes being unable to read from the directory.

So, this is a problem with incorrect $SAVEGAMEDIR$ path on MAC?

When you say "crashes being unable to read from the directory" here are you still referring to the same "null pointer" error when accessing the dynamic sprite pointer, or is it another error?

morganw

Quote from: Crimson Wizard on Mon 17/10/2022 15:50:14Linux and Mac have case-sensitive filepaths
It is file-system dependent, but I believe the default one on macOS is case insensitive. i.e. if the problem was the case then macOS and Windows would likely be doing something similar and a Linux system would be doing something different.

Crimson Wizard

#16
I finally had time to test this properly, and I confirm that DynamicSprite.CreateFromFile does not work on Linux, regardless of the path.

File.Open does work for exactly same file path.

This means that either:
- DynamicSprite.CreateFromFile does or does not do something that File.Open does (to path or to file);
- The bitmap loading is broken on Linux/OSX.

The script I used for the test:
Code: ags
// room script file

function TestLoadImage(String filename)
{
	if (File.Exists(filename)) {
		Display(String.Format("%s detected", filename));
	} else {
		Display(String.Format("%s NOT detected", filename));
	}

	File *f = File.Open(filename, eFileRead);
	if (f != null) {
		Display("Opened %s", filename);
		f.Close();
	} else {
		Display("Failed to open %s", filename);
	}

	DynamicSprite *dspr = DynamicSprite.CreateFromFile(filename);
	if (dspr != null) {
		Display(String.Format("Sprite was created from %s", filename));
		dspr.Delete();
	} else {
		Display(String.Format("Failed to create sprite from %s", filename));
	}
}

function room_AfterFadeIn()
{
	TestLoadImage(String.Format("saveHD%d.bmp", 0));
	TestLoadImage(String.Format("$SAVEGAMEDIR$/saveHD%d.bmp", 0));
}

First test tries to open file / create image in the game's directory, another in the save dir. Result is the same (assuming you have bmps in both).

For both paths the Windows results:
- saveHD0.bmp detected
- Opened saveHD0.bmp
- Sprite was created from saveHD0.bmp

The Linux results:
- saveHD0.bmp detected
- Opened saveHD0.bmp
- Failed to create sprite from saveHD0.bmp

Crimson Wizard

#17
After more in-depth debugging, this seem to be an issue with a PACKFILE reading again.

Here where it's done in the allegro4 library:
https://github.com/adventuregamestudio/ags/blob/fbb87c3370501d9acad9c7282c5e951032587c9e/libsrc/allegro/src/bmp.c#L594-L616

If I add a direct file reading using `fread`, then it reads the bitmap header perfectly.
But the allegro's PACKFILE functions read some garbage instead, so the function fails.

In this case the allegro is using its default PACKFILE reading operations. In other cases we provide our custom readers, and they work...
Either something got broken recently, or this is again a consequence of non-careful stripping of allegro.

PS. I think one quick solution could be to actually open our custom packfile for this too; except we will have to detect format from extension ourselves.
EDIT: hmm, to think of it, it should use a custom packfile with our streams anyway, because otherwise it won't be able to read image files stored in Android package.

PPS. But of course one thing that is still confusing is why would this work on Windows and not on Linux/OSX.
So, this might have something to do with 32/64-bit differences, or standard library differences (again, the types and sizes of variables).

eri0o

#18
@Crimson Wizard do you want to try the bitmap streamer road again? I think it was almost there except for some palette conversion stuff.

Crimson Wizard

#19
Quote from: eri0o on Tue 18/10/2022 09:38:38@Crimson Wizard do you want to try the bitmap streamer road again? I think it was almost there except for some palette conversion stuff.

No; not in 3.6.0 at least, and there was also PCX format support missing in that replacement, which might be necessary for backwards support.
Overall, I'd rather wish we used some image library that does multiple format reading and writing to save on code maintenance, and only make our own format reading as a last resort.

SMF spam blocked by CleanTalk