ME3Tweaks / LegendaryExplorer

Editor toolset for Mass Effect Trilogy and Mass Effect Legendary Edition
https://me3tweaks.com
GNU General Public License v3.0
125 stars 29 forks source link

Sequence Editor fails to handle SequenceReference-s referencing imports #190

Closed 4rterius closed 3 years ago

4rterius commented 3 years ago

Describe the bug

Using Sequence Editor to open ME1 files which contain sequences with SequenceReference-s referencing imported sequences fails with a message box (see screenshot).

To Reproduce

  1. Open Sequence Editor
  2. File > Open
  3. Select a file with SequenceReference-s referencing imported sequences, e.g.:
    • Mass Effect\BioGame\CookedPC\Packages\GameObjects\Sequences\BIOG_World_Q.upk
    • Mass Effect\BioGame\CookedPC\Packages\GameObjects\Sequences\BIOG_NormandyAirlock_Q.upk
  4. Click 'Open'

Expected behavior

Sequence Editor loads the sequences and otherwise works normally.

Screenshots and files used to replicate this

Error: image

Version information:

4.2 Debug, built from Beta branch 10/6/2020

Stack trace

N/A

Other information

After some investigation, I found out that the error happens because Sequence Editor does not expect referenced sequences to not be an export. Due to my limited knowledge of how Sequence Editor or UE works I'm not sure to what extent imported referenced sequences can be supported, but simply adding branches to handle sequence references with oSequenceReference < 0 seems to do the trick and at least allow for everything else to be loaded and displayed. Not sure how correctly though.

Attaching a diff and a screenshot of a kinda-working SeqEd, if the changes I've made meet your approval I can open a PR.

Diff of proposed changes ```diff diff --git a/ME3Explorer/Sequence Editor/SequenceEditorWPF.xaml.cs b/ME3Explorer/Sequence Editor/SequenceEditorWPF.xaml.cs index 9bbc8a07..13eb2a95 100644 --- a/ME3Explorer/Sequence Editor/SequenceEditorWPF.xaml.cs +++ b/ME3Explorer/Sequence Editor/SequenceEditorWPF.xaml.cs @@ -404,8 +404,15 @@ namespace ME3Explorer.Sequence_Editor if (SetProperty(ref _selectedItem, value) && value != null) { - value.IsSelected = true; - LoadSequence((ExportEntry)value.Entry); + if (value.Entry.UIndex > 0) + { + value.IsSelected = true; + LoadSequence((ExportEntry)value.Entry); + } + else + { + MessageBox.Show("Can't select an imported sequence"); + } } } } @@ -435,7 +442,7 @@ namespace ME3Explorer.Sequence_Editor { LoadFile(d.FileName); } - catch (Exception ex) + catch (Exception ex) when (!App.IsDebug) { MessageBox.Show(this, "Unable to open file:\n" + ex.Message); } @@ -484,7 +491,7 @@ namespace ME3Explorer.Sequence_Editor variablesToolBox.Classes = SequenceObjectCreator.GetSequenceVariables(Pcc.Game).OrderBy(info => info.ClassName).ToList(); } - catch (Exception ex) + catch (Exception ex) when (!App.IsDebug) { MessageBox.Show(this, "Error:\n" + ex.Message); Title = "Sequence Editor"; @@ -577,9 +584,22 @@ namespace ME3Explorer.Sequence_Editor var propSequenceReference = exportEntry.GetProperty("oSequenceReference"); if (propSequenceReference != null) { - TreeViewEntry t = FindSequences(pcc.GetUExport(propSequenceReference.Value)); - SequenceExports.Add(exportEntry); - root.Sublinks.Add(t); + if (propSequenceReference.Value > 0) + { + TreeViewEntry t = FindSequences(pcc.GetUExport(propSequenceReference.Value)); + SequenceExports.Add(exportEntry); + root.Sublinks.Add(t); + } + else + { + // TODO: handle import + var import = pcc.GetImport(propSequenceReference.Value); + var importTreeViewEntry = new TreeViewEntry(import, $"#{import.UIndex}: {import.InstancedFullPath}") + { + IsExpanded = false + }; + root.Sublinks.Add(importTreeViewEntry); + } } } } diff --git a/ME3Explorer/Sequence Editor/SequenceObjects.cs b/ME3Explorer/Sequence Editor/SequenceObjects.cs index 7b63360e..5dd9d4f9 100644 --- a/ME3Explorer/Sequence Editor/SequenceObjects.cs +++ b/ME3Explorer/Sequence Editor/SequenceObjects.cs @@ -1569,7 +1569,15 @@ namespace ME3Explorer.SequenceObjects var oSequenceReference = export.GetProperty("oSequenceReference"); if (oSequenceReference != null) { - inputLinksProp = pcc.GetUExport(oSequenceReference.Value).GetProperty>("InputLinks"); + if (oSequenceReference.Value > 0) + { + inputLinksProp = pcc.GetUExport(oSequenceReference.Value).GetProperty>("InputLinks"); + } + else + { + // TODO: handle import + Debug.WriteLine($"Can't get input links of {export.InstancedFullPath} because it references an import."); + } } } ```
Screenshot of kinda-working SeqEd ![image](https://user-images.githubusercontent.com/16708023/95133623-43c76180-076a-11eb-921e-0ce2fcb5d4b5.png)
Mgamerz commented 3 years ago

Changes make sense, however you can test if it is a positive value on IEntry objects by testing is ExportEntry. Export entires always have a positive UIndex and imports always have a negative UIndex. I suggest switching to librarysplit branch as that is where all of our work is currently going and it is going to be merged into Beta soon, but might take a few days. Beta is kind of stale for a few months now.

Test if it is an export by doing if (value.Entry is ExportEntry export). There is also methods on IMEPackage for TryGetExport I beleive that you can use to try to get a value. If you use this with if statement, you can determine if you have a valid object to load.

If you open a PR, open it against the librarysplit branch and we will review it. Your changes seem reasonable.

4rterius commented 3 years ago

Great, thanks, will do!

SirCxyrtyx commented 3 years ago

closed by #196