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.

Sign up for GitHub

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

Jump to bottom

Add analysis for reference validity #107

Closed
focustense opened this issue Sep 1, 2021 · 0 comments
Closed

Add analysis for reference validity #107

focustense opened this issue Sep 1, 2021 · 0 comments
Labels
easynpc Issues/requests relating the EasyNPC app enhancement New feature or request usability Difficult to use, requires design revisions

Comments

@focustense
Copy link
Owner

A substantial number of issues and support questions revolve around plugins with invalid form IDs. To be blunt, while I have nothing against people who edit plugins (I myself did a lot of this back in my newbie modding days), they waste a lot of time and energy that could be spent on fixing real bugs or improving the app.

Most parts of the app will ignore invalid references, but some really can't, namely the build pipeline. If an NPC is supposed to reference - for example - a head part, texture set, armor, etc., which doesn't exist, then it can't be imported and the final mod is going to be broken no matter what.

I propose to pull this into the analysis phase. While it would be far too costly to follow every single reference - the branching is crazy when you get to records like Race - the idea would be to declare specific paths and chains of paths that we know for certain will need to be followed, and have the analysis report any that can't be completed, probably as a flat list with paths as a string or list.

Then, lock out the entire plugins, or at least the NPC records, so that they cannot be selected as profile options either manually or during a profile restore/import, and show this as a warning at startup.

While this will probably lead to a different and hopefully smaller set of complaints about the warnings, that is an easier problem to handle because the warnings will clearly indicate what the problem is - i.e. does not require analysis of a crash log.
@focustense focustense added enhancement New feature or request usability Difficult to use, requires design revisions easynpc Issues/requests relating the EasyNPC app labels Sep 1, 2021
focustense added a commit that referenced this issue Sep 2, 2021
@focustense
Add reference-checking utility.
f6dd9be 
Can be configured with expressions pointing to all or most common record expansions, and will follow all paths and report the full paths from the root record to any unresolvable references.

This should help to produce useful, user-friendly (to the extent that is possible) output about plugins that have these problems and exactly why they are problems, as well as pointing directly to the records (i.e. NPCs) that need to be suppressed.

Still needs to be incorporated into the other analyzers.

#107
focustense added a commit that referenced this issue Sep 2, 2021
@focustense
Integrate ref-checking with analyzers and app.
c4e9419 
All analyzers now optionally take a reference checker and will execute it, and the unit tests enforce that every analyzer does support and properly use the parameter.

EasyNPC app now configures the full import expansions for NPCs - that is, all of the possible paths that can cause a build to fail. For now, these are only reported as warnings in the log. They'll need to be turned into a decent UI.

Note: This adds a non-trivial amount of startup time, perhaps between 0.5-1 sec. Consider adding a command-line flag for advanced users to turn it off.

#107
focustense added a commit that referenced this issue Sep 4, 2021
@focustense
Refactor nav view to use MVVM (mostly).
e7bc087 
With more potential interstitials and "out-of-flow" views, the hacked-together code-behind design isn't scaling well.

Switch to a data binding and data template pattern, accommodating for the few bugs and idiosyncracies in ModernWpf as best we can.

#107
focustense added a commit that referenced this issue Sep 4, 2021
@focustense
Disable and warn on invalid references.
01e96b4 
When NPCs in plugins dead-end at some broken form link, they will be shown in an error report at startup for prominence, and subsequently disabled in all selection functions and marked disabled in the UI. This will prevent what would otherwise appear to be either a crash on build (current behavior) or some undefined and probably wrong behavior (if we ignored them).

Considering an "ignore permanently" but not sure if the benefits outweigh the potential risk of users just clicking anything they can to get rid of the errors without paying attention.

Still need doc page which is linked by the warning, will close after that's done.

#107
focustense added a commit that referenced this issue Sep 5, 2021
@focustense
Apply load order to invalid refs report.
0768ead 
Was finding it annoying how the order could change each time due to the parallel implementation. `AsOrdered` wasn't helping.

#107
This was referenced Sep 6, 2021
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
easynpc Issues/requests relating the EasyNPC app enhancement New feature or request usability Difficult to use, requires design revisions
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@focustense