2shady4u / godot-sqlite

GDExtension wrapper for SQLite (Godot 4.x+)
MIT License
895 stars 79 forks source link

Handle unicode text #90

Closed lightyears1998 closed 1 year ago

lightyears1998 commented 1 year ago

This should fix #87.

I have tested the following cases on Windows:

Note: import_from_json and export_to_json won't work with non-ASCII file paths on Windows. It is the expected behavior since we are using ifstream/ofstream directly. I notice that open_db could work with the non-ASCII file path, probably because sqlite3_open_v2 handles the file path for us. It seems that there is no easy solution for Unicode path handling on Windows.

image

2shady4u commented 1 year ago

Hi @lightyears1998

Apologies for the delay! I've been quite busy with other things 😞

Am I correct in assuming that this fix would only work correctly when the database is encoded in UTF8? According to the SQLite documentation, it should also be possible to create a database that uses UTF16.

Unfortunately I seem to keep getting an out of memory-error when I attempt to create a UTF16 database... (Which might be another problem entirely since I'm getting the same error on the gd-extension-branch)

EDIT: It seems to work even with a UTF16 encoded database πŸ˜„

lightyears1998 commented 1 year ago

Never mind. You can certainly take your time. (BTW, thanks for the awesome project!) πŸ‘

I think this fix should work no matter what the database text encoding is. I make the following test, and it seems to work correctly.

Minimal Demo Code ``` gdscript # database.gd (autoload) extends Node var db = SQLite.new() func _ready() -> void: db.path = "res://database.sqlite3" db.verbosity_level = SQLite.VERY_VERBOSE db.open_db() db.query("PRAGMA encoding = 'UTF-16BE'") # 'UTF-16LE' also works for me. db.create_table("entry", { "id": { "data_type": "int", "primary_key": true, "auto_increment": true }, "title": { "data_type": "text" }, "content": { "data_type": "text" } }) db.insert_row('entry', { "title": "The quick brown fox jumps over the lazy dog", "content": "ζˆ‘θƒ½εžδΈ‹εΌΉη θ€ŒδΈδΌ€θΊ«δ½“γ€‚" }) db.query("select * from entry") print(db.query_result) ``` Here is a screenshot from sqlite3 CLI showing the database text encoding: ![image](https://user-images.githubusercontent.com/22541353/190293407-62c2fb3d-16b4-4a71-a459-fb3d903eb550.png)

(I am not sure about the out-of-memory error, but making a query before open_db() could lead to such an error?)

As documented in sqlite3 document, sqlite3_errmsg, sqlite3_expanded_sql, sqlite3_column_text and sqlite3_column_name return a pointer to an UTF-8 string. (There are also UTF-16 variations of these functions, for example, sqlite3_column_name16.) I guess that the database text encoding is processed interally in the db, and text is conveted to UTF-8 before returning from these functions. Godot editor use UTF-8 encoding by default, so the string literals in the source code is encoded in UTF-8 in most cases. (One has to re-compile the editor and export templates to change the text encoding of godot.) For the above reasons, I think the fix is feasible.

lightyears1998 commented 1 year ago

Unfortunately I seem to keep getting an out of memory-error when I attempt to create a UTF16 database... (Which might be another problem entirely since I'm getting the same error on the gd-extension-branch)

Another memory related issue is the one I issued in the godot-cpp repo. As that issue infects CharString, perhaps it is somehow related to our issue, and we have to wait for upstream to fix the issue before ours can be resolved.

The Long Story My game uses code from this repo as a database addon, and it works in the editor and in the debug build, but crashes in the release build at start up without any error messages or warnings. To locate the problem, I first checked my code and removed everything that could be problematic. However the crashes reamined. Then, I looked into this repo and tried to find something buggy. Eventually, I found the code written well. It took me around 12 hours to find out there seems to be a problem in the upstream Godot-CPP headers. 🀣 It's so good to locate where the problem might be, though the procedure is nearly getting me crazy.
2shady4u commented 1 year ago

Hi @lightyears1998

As this fix also works for UTF-16, I do think it is ready to be merged πŸ˜„ Thank you for your contribution!