Skip to content

Conversation

@yonilerner
Copy link

@yonilerner yonilerner commented Dec 22, 2024

Hello! I didnt see any documentation about contributing, so sorry in advance if this isnt correct. Anyway, let me tell you about this PR.

Ive been using this tool a lot lately, and on larger SAV files, many operations completely lock up the UI, sometimes for up to 30 seconds, and windows starts to complain very quickly that the program is not responding. And having the UI completely locked up is a pretty bad experience in general.

So the goal of this PR is to take certain long running operations and move them off the main UI thread. I demonstrate a primitive task system that does a few things:

  • Creates a thread pool
  • Allows you to send tasks to that pool
  • Calls the UI render loop while the thread is executing the task
  • [Optionally] Locks certain parts of the UI during task execution to ensure dangerous operations cant happen
  • Returns the result of the task

Here's a quick video of what it looks like to use this program with these changes:

ySBAuBV.mp4

This is very much a proof of concept, but I wanted to get some early feedback on the approach before I kept working on it. I only implemented Loading and Saving menu operations right now, but in theory any long running task could be migrated to this system.

I think it could be merged in its current form, but there are a few changes I'd make to improve it:

  • Ensure tasks cant conflict with each other (probably create specific task labels and only allow one running instance per label)
  • Ensure tasks cant operate on shared resources (probably just locks)
  • Potentially disable more UI elements during tasks if using them could be risky; Im not sure what these could be, open to feedback

Let me know what you think!

@EternalWraith
Copy link
Owner

There's a few changes you've made to the code that appear to be targeted just to your local machine. For example, the default open file location is directly being routed to a location on your harddrive.

@yonilerner
Copy link
Author

@EternalWraith is it possible youre looking at the first commit only? I removed that in the second commit

@EternalWraith
Copy link
Owner

My bad, It seems I was looking only at the first commit. I will work on integrating this into the current version of PalEdit as unfortunately it seems that there are currently branch conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants