search

shpaass / yafc-ce

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

Delete page sometimes only hides it #147

Closed Dorus closed 3 months ago

Dorus commented 3 months ago

afbeelding

The delete page button above does not work for some pages, but does work for others. Need to dig into this to find out why.

Dorus commented 3 months ago

While studying my local project with this issue, I noticed that for many tabs, the property canDelete of my YAFC.Model.ProductionTable is false. However, checking the code, it should only ever be false for YAFC.Mode.Summary so i'm not sure how my local project got in that state.

Dorus commented 3 months ago

Ah i think i found it:

https://github.com/have-fun-was-taken/yafc-ce/commit/212f93848162395ba72d17b203d0182a21049c06

This change made existing pages impossible to delete, since 'canDelete' will be false when the boolean is absent.

veger commented 3 months ago

Hm... I messed up by picking a new field where the default (false) is wrong. So on existing saves (from before this change) the default is wrong, as for those pages the canDelete should have been true...

I do not think it is possible to fix this anymore, as we do not have file versions, so we cannot detect whether the fix was applied on a save already or not.

I suppose the easiest way to fix a save, is to open the file in your favorite text editor and manually change the canDelete values to true (except for the YAFC.Model.Summary as this field was introduce for this).

veger commented 3 months ago

Now that I think about it, if the SummaryPage would support getting deleted (I forgot why this is not the case). The whole canField field can be removed and the issue would be solved as well/properly.

Dorus commented 3 months ago

Yes, i think we can just find out if it's a summary page another way, like with page.contentType == typeof(Summary), but if SummaryPage gets delete support, you could get rid of the entire logic. What would you prefer? I could make PR for either scenario, but i'm not sure if supporting deletino for SummaryPage require additional work.