Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

emscripten: signal when data is written (so it persists) #1631

Closed
wants to merge 1 commit into from
Closed

emscripten: signal when data is written (so it persists) #1631

wants to merge 1 commit into from

Conversation

@Beuc
Copy link
Contributor

@Beuc Beuc commented Nov 10, 2018

The virtual filesystem is transient until an asynchronous sync message is signaled.
This patch does it when the user saves their game.

@renpytom
Copy link
Member

@renpytom renpytom commented Nov 10, 2018

I'm holding off until 7.1.2 is out to merge this.

That being said, I'm wondering if it might make sense to deal with this at some sort of lower level - like through a patch to Python. It just feels really wrong to annotate Ren'Py with code that just brings things into compliance with the way Python is documented to work, you know?

Can you call the emscripten stuff from C? It should be easy to check to see if a Python file object is read-write, and call if from there.

@Beuc
Copy link
Contributor Author

@Beuc Beuc commented Nov 11, 2018

Hi,

This is necessary to synchronize the ~/.renpy/ mount point only. The root filesystem is in transient.
Reference for that function is at https://kripken.github.io/emscripten-site/docs/api_reference/Filesystem-API.html
It would not make sense to call syncfs e.g. everytime a line is written to log.txt.
Also architecturally, doing syncs purely at the Python level means we can miss syncs in C modules.

The emscripten module is my C wrapper:
https://www.beuc.net/python-emscripten/python/artifact/a308c76692f5c513
https://github.com/python-emscripten/python/blob/trunk/emscripten.pyx
so everything is called from C.

renpytom added a commit that referenced this pull request Dec 10, 2018
This is a rewrite of #1631, to put all the emscripten changes into
a single function.
@renpytom
Copy link
Member

@renpytom renpytom commented Dec 10, 2018

I rewrote this in 372216e, to route all of this through a single method.

@renpytom renpytom closed this Dec 10, 2018
@Beuc
Copy link
Contributor Author

@Beuc Beuc commented Dec 13, 2018

I had made a few changes since then to avoid doing many syncs during autosave (which does around 10 renames in a row) - i.e. no sync on rename, and sync at the end of autosave_thread.
This doesn't fit defining sync in FileLocation; I'll probably write another patch unless you want to proceed differently.

@Beuc Beuc deleted the Beuc:patch-6 branch Dec 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants