Zylann / godot_voxel

Voxel module for Godot Engine
MIT License
2.69k stars 251 forks source link

Keep getting this error code. #365

Closed probedrone closed 2 years ago

probedrone commented 2 years ago

I keep getting this error in the voxel game project as well as my own game project. I have no idea what to do about it. image

Zylann commented 2 years ago

Duplicate of https://github.com/Zylann/godot_voxel/issues/360

Which version of the module? Which OS? Which compiler?

Some info if you want to help troubleshooting this issue: https://github.com/Zylann/godot_voxel/issues/360#issuecomment-1079436156

probedrone commented 2 years ago

Both the 3.x and 4.0 branches give the issue, using the versions built using Github action. My OS is Windows 10.

Zylann commented 2 years ago

If you got it with Godot 4.0, what was the logged message there? The screenshot you posted seems to be from Godot 3.

After fixing a few issues due to breaking changes in Godot4 and trying with this version on Windows 10, I launched blocky_game (godot4 branch), walked around for a few minutes, saw the save being generated, didn't see the error. Not saying it got fixed or anything, I just wasn't able to reproduce it to this day.

probedrone commented 2 years ago

Trying with the issue you used it works on my side with the Godot4 branch now. It must of just been because I was using an older version of the 4 branch without the fixes is my guess?

Zylann commented 2 years ago

So far it's hard to say what the origin of the issue even is, so can't really say wether or not anything got fixed^^"

probedrone commented 2 years ago

The issue tho seems to persist in the 3.X branch of godot however.

Zylann commented 2 years ago

3.x is using pretty much the same code so I have no idea why that would happen there, apart from that sizeof(int) you could change to a sizeof(uint32_t) https://github.com/Zylann/godot_voxel/blob/726bd807506cef7f7e8ee34ba774ec793ec5f034/streams/region/region_file.cpp#L367 And if that doesnt fix anything, if you look at the full snippet for godot3.x, I dont see anything else wrong with the code, apart from Godot potentially having a bug of some sort https://github.com/Zylann/godot_voxel/issues/360#issuecomment-1079436156

probedrone commented 2 years ago

An additional note to this, I can confirm that both the Mono and Windows build give the error

Zylann commented 2 years ago

I just tried the same process with the godot3.x branch and the master branch of the voxel demo from Github, launched it, walked around and blew up some blocks for 5 minutes, no issue occurred. You might need to explain more precisely how exactly you are reproducing this from scratch.

For now, I ported the change to sizeof(uint32_t) to godot3.x and added a few more details to the error message.

probedrone commented 2 years ago

Tried testing the latest build and still received the error. I noticed it happens when I walk a certain amount away from spawn in it. Here is the full debugger log. image

Zylann commented 2 years ago

Do you keep testing this with the same project without redownloading a clean copy? Also do did you enable "run stream in editor" on the terrain? (hint: don't)

I inverted the logged info, but if corrected it basically shows the following:

f here is FileAccess, a Godot class. I still can't reproduce this even in Godot 3, even walking hundreds of blocks away from origin...

I pushed some changes to use uint64_t also in offsets and added more checks, but like before I doubt the issue will get fixed. At best it can be pinned there but it would need investigation by someone who can get a reliable repro.

probedrone commented 2 years ago

I tested with a clean copy and everything. I have an idea that it may have to do with how it is trying to save the data and it is getting corrupted somehow?

Zylann commented 2 years ago

The question is, how can it possibly get corrupted when no multithreading is at stake (there is only one thread running that code), and why so subtly? Like, it's not very wrong or even crashy, it's always just slightly off, and only when some people test it, so far only in Godot 3. My own guess is that somehow the OS interprets the buffer as text, for example it might convert LF into CRLF which is two more bytes. And there was a bug with get_position in Godot 3.3 but it got fixed, and that file is opened in binary mode (or rather should be, Godot doesnt have such option).

probedrone commented 2 years ago

This is all rather baffling indeed. I was hoping to use the voxel plugin for my own VR RPG game

Zylann commented 2 years ago

You don't have to use VoxelStreamRegionFiles tho, VoxelStreamSQLite works too (that one only needs a true path, without the res://)

probedrone commented 2 years ago

Weird, in VoxelStreamRegionFiles, if I keep the path field blank I get no issues with it? Update: Disregard last edit to this. Still getting the error after tabbing back into the game with path form filled after changing system locale.

Zylann commented 2 years ago

Do you keep getting the same message in the error? My last commits added more checks around store_buffer. Also does your terrain have the setting run_stream_in_editor enabled?

probedrone commented 2 years ago

I have run_stream_in_editor disabled. Here is the issue it is coming with now when the path field is filled. image

Zylann commented 2 years ago

Yeah at this point there is nothing else I can do... store_buffer is pretty much not working as expected. It writes 29 more bytes for a buffer size of 868.

https://github.com/Zylann/godot_voxel/blob/9d818fa4b0b7dd34ccae3f9ad82e38ecf2bceabb/streams/region/region_file.cpp#L369-L381

To debug this further I would need to be able to reproduce this issue and I can't... what I'm curious to see now is what that buffer look like. What was supposed to be written, and what was actually written, to see if there is a pattern going on.

Out of curiosity, what happens if you run this project using the same Godot version? StoreBuffer.zip It's not exactly the same test so it might be for nothing, but if it reproduces there it could save time.

probedrone commented 2 years ago

Getting this error in the StoreBuffer project. image

Zylann commented 2 years ago

That's the call stack, not the error. But you shouldn't get any error. When I test with this build, I don't get any. Are you able to get the same error with an official Godot version?

probedrone commented 2 years ago

After extensive testing, I do not get any errors with the StoreBuffer test in other projects.

Zylann commented 2 years ago

What do you mean "in other projects"? StoreBuffer is a project. You mean with other Godot versions?

probedrone commented 2 years ago

Yeah I meant other Godot versions. I also noticed that now the issue only comes up in the Mono build of Godot after the fixes?

Zylann commented 2 years ago

After which fixes? And which Mono build?

probedrone commented 2 years ago

After switching to the uint64_t fix the regular Godot Voxel build works but not the latest Mono Godot Voxel build. I been using Mono this whole time since I need C# for the more intense number crunching for AI and RPG stats in my own game.

Zylann commented 2 years ago

I have no idea why Mono makes a difference here... it was already a nightmare to make the CI produce working Mono builds^^" Maybe try compiling Godot yourself, in case it makes a difference somehow...

After testing more in the test project, I can confirm get_position() after a store_buffer() seems to have counted bytes of value 10 (\n in ASCII) as if they were two bytes instead of one. So if you store a buffer containing 10 times \n, get_position() returns 20. Even though the file is not even open in text mode. There isn't even a text mode in Godot's API.

probedrone commented 2 years ago

Any luck on finding a way to fix this issue? I am honestly stumped over it.

Zylann commented 2 years ago

Nothing new, sorry. Have you tried building a Mono version yourself?

Zylann commented 2 years ago

I'll close this issue because it was a duplicate. I'll link it in https://github.com/Zylann/godot_voxel/issues/360#issuecomment-1094430575 because it contains some insight.