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

Incineratez/memoryleak #377

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

IncinerateZ
Copy link

Bulk exporting properties cause a huge memory leak.

I'm not experienced in C# so I just moved stuff that I think caused it around.

@4sval
Copy link
Owner

4sval commented Apr 3, 2023

It looks like the actual change was not to display the JSON on save.

While this is a good idea to save resources, the main purpose of FModel is to display information. It has a GUI for a reason and is not meant to be the most efficient/fastest to "bulk do work here". After all, CUE4Parse is separate from FModel.

I know for a fact that some people use these bulk options because all they want is one thing and don't wanna spend time looking into where it is. So they go up enough in the tree, right-click, left-click, and wait.

Against my will, we are slowly moving toward that kind of usage, but why not. However, I think the idea behind this PR can be pushed further. Like not displaying anything after a threshold on the number of packages to deal with, and overall, more context-aware processes. We're still working with a lot of left-over from the auto "do work here" era. A context class will certainly help with readability and maintainability.

This PR won't be merged as is, but I'll keep it open for the time being.

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