shpaass / yafc-ce

Powerful Factorio calculator/analyser that works with mods
GNU General Public License v3.0
54 stars 20 forks source link

Remove canDelete property so that non-summary pages can always be deleted. #156

Closed Dorus closed 3 months ago

Dorus commented 3 months ago

Fix #147

Dorus commented 3 months ago

afbeelding Now hitting this code on my test project. Not sure if it's related. Will need to check.

Dorus commented 3 months ago

I've reduced the above mentioned problem to 3 lines of

            {
              "multiplier": 1,
              "page": null,
              "subgroup": {
                "elements": [],
                "expanded": false,
                "name": null
              }
            },
            {
              "multiplier": 1,
              "page": null,
              "subgroup": {
                "elements": [],
                "expanded": false,
                "name": null
              }
            },
            {
              "multiplier": 1,
              "page": null,
              "subgroup": {
                "elements": [],
                "expanded": false,
                "name": null
              }

In one of the YAFC.Model.ProductionSummary -> content.group.elements items. However saving the file again fixes it and i cannot figure out how i made this happen.

I'm running in some other issues while testing this, will report those separate since they're also on master. Meaning it's unrelated to this PR and this PR could be merged.

shpaass commented 3 months ago

saving the file again fixes it

In addition to the changes in the PR, what do you think about a patching mechanism for the saves? For instance, if YAFC detects that the project file was updated earlier than the date this fix was released, then it makes a backup and re-saves the file.

veger commented 3 months ago

The 3 elements you showed do not have undefined subgroups? So subgroup == null should be false and the exception should not have been thrown?

TBH I doubt that this is related, as canDelete has nothing to do with subgroups. Maybe the file got corrupted because (some of) the pages were partially deleted, which now gets triggered since canDelete is not (incorrectly) guarding this anymore?

Dorus commented 3 months ago

saving the file again fixes it

In addition to the changes in the PR, what do you think about a patching mechanism for the saves? For instance, if YAFC detects that the project file was updated earlier than the date this fix was released, then it makes a backup and re-saves the file.

Even the master branch fixes it on save. So the cause of that problem was either me manipulating the process in the debugger, or introducing it in one of my earlier changes, or it might even have been something in the older master before i rebased. Either way i cannot reproduce it anymore both on master and this branch.

The only thing i can do, is introduce some robustness in YaFC when it hits the notsuportedexception, when i step past the notsuportedexception in YaFC with the debugger and save again, the problem fixes itself, so that's something i could patch in. But that seems more appropriate for a separate pr. (Sorry i should have mentioned it only fixes it on saving if i manipulate the program in de debugger during loading, else it wont load the file at all)".

Tl;dr: i dont think it's a problem introduced by the current pr. I could fix the loading code, but would do so in a separate pr.

shpaass commented 3 months ago

Rebased on fresh master