SaveScum / skyrim-plugin-decoding-project

Automatically exported from code.google.com/p/skyrim-plugin-decoding-project
0 stars 0 forks source link

Uncatched AV when copying REFR with XLOC subrecord in TES4Edit #136

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Copy [REFR:00030210] (places BravilLoadDoorLower01 "Door" [DOOR:0002FF95] in 
GRUP Cell Persistent Children of [CELL:00003615] (in BravilWorld "Bravil" 
[WRLD:0001C319])) to a new file in TES4Edit

What is the expected output?
ITM record

What do you see instead?
Uncatched AV exception.

What version of the product are you using? On what operating system?
r1348

Please provide any additional information below.
There are several similar subrecords using wbUnion with decider based on 
DataSize, neet to check them too.

Original issue reported on code.google.com by zila...@gmail.com on 30 Jun 2013 at 3:37

GoogleCodeExporter commented 9 years ago
It looks like this calls a decide which ends up calling itself, and round and 
round.
I have to roll back or adapt the changes to decide in 3.0.31

Original comment by HuguesLe...@gmail.com on 30 Jun 2013 at 8:06

GoogleCodeExporter commented 9 years ago
The issue is we have a decider that calls GetSize when GetSize calls Decide now 
since I introduced GetDefaultSize ! I have to revert to hardcoding default 
union size to udDecider[0].Size otherwise that puts a serious limitation on 
what a Decider can do. Thougths ?

Original comment by HuguesLe...@gmail.com on 30 Jun 2013 at 8:53

GoogleCodeExporter commented 9 years ago
That's why I asked Varians in forum to check flags position in XLOC subrecord, 
I tried to find some other dependency for decider beside DataSize. But they are 
none.
I checked all the unions that use decider based on DataSize, and they all are 
getting the size of subrecord they are in, and that value is always known 
beforehand since it is in subrecord header. So basically those deciders don't 
need a dynamic or default size of container, all they need is value from 
subrecord header stored in esp/esm.
I don't remember if that that value is exposed or even stored anywhere by 
xEdit. If not, then maybe add it so deciders can be rewritten as

function wbXLOCFillerDecider(aBasePtr: Pointer; aEndPtr: Pointer; const 
aElement: IwbElement): Integer;
var
  Container: IwbContainer;
begin
  Result := 0;
  if not Assigned(aElement) then Exit;
  Container := GetContainerFromUnion(aElement);
  if not Assigned(Container) then Exit;

  if Container.StoredSize = 16 then
    Result := 1;
end;

As for reverting back to udDecider[0].Size, I don't remember what issue was 
fixed with introduction of GetDefaultSize? Or it is only an optimization?

Original comment by zila...@gmail.com on 1 Jul 2013 at 5:15

GoogleCodeExporter commented 9 years ago
DefaultSize solved issue 134.

Original comment by HuguesLe...@gmail.com on 1 Jul 2013 at 10:33

GoogleCodeExporter commented 9 years ago
I went with IwbSubRecord.SubRecordHeaderSize property.

Original comment by HuguesLe...@gmail.com on 1 Jul 2013 at 10:47

GoogleCodeExporter commented 9 years ago
Should be solved as of r1349

Original comment by HuguesLe...@gmail.com on 1 Jul 2013 at 5:23

GoogleCodeExporter commented 9 years ago
Thanks.
I think it is time to write a "test case" script for all game modes which will 
copy/modify/add/remove records and subrecords while checking for errors and 
catching exceptions. Even closed testers can't catch all errors anymore.

Original comment by zila...@gmail.com on 1 Jul 2013 at 6:30

GoogleCodeExporter commented 9 years ago
Check copying of all records involving new deciders, most are benign (between 
wbEmpty and something else), but several AI Packages in FO3 and FNV are not 
ITM. Quite obvious since subrecord header size is 0 for new subrecords. Saving 
and reloading fixes it.
I don't know really what is the best approach here: leave as is since copied 
data is correct or copy srsHeaderSize to new subrecords.

Original comment by zila...@gmail.com on 2 Jul 2013 at 7:44

GoogleCodeExporter commented 9 years ago
Unless there is an issue with editing it before saving, I would not worry about 
any "non ITM" that would be solved by simply saving and reloading.
But anyway I will see if duplicating the srsHeaderSize is possible. I am/will 
be looking at all the copy /assign functions soon for an idea I had about 
"merging". As always the issue is detecting it is a copy and not a new item.

Original comment by HuguesLe...@gmail.com on 2 Jul 2013 at 1:10

GoogleCodeExporter commented 9 years ago
Those non ITMs are only for 10-15 packages and only in FO3 and FNV, nothing 
serious.
"Merging"? Sounds exciting :)

Original comment by zila...@gmail.com on 2 Jul 2013 at 4:41

GoogleCodeExporter commented 9 years ago
First, it won't be called merging :)
For now I use Move and Deep move.
The idea is to be able to move records from individual plugins into a resource 
master upper in the load order. 
Kind of the opposite of merge patch or Bash patch. :)
That is how I manually do today, but automating it should make it both easier 
and compatible with updating the original plugin.

Original comment by HuguesLe...@gmail.com on 5 Jul 2013 at 2:37

GoogleCodeExporter commented 9 years ago
Are you creating something like wbCopyElementToMaster()? It will indeed be very 
handy when writing a plugin merger script.

Original comment by zila...@gmail.com on 5 Jul 2013 at 4:05

GoogleCodeExporter commented 9 years ago
What I want initially is a compound Copy as new record, followed by Change 
FormID for the initial import. Recursive so any form referenced "between" the 
target and the source would be copied as well.

On update, I wish to merge the changes based on EditorID.

Obviously not 100% would work, like any FNV MCM processing. But for what is 
usually placed into a "resource mod" it should work.

Original comment by HuguesLe...@gmail.com on 5 Jul 2013 at 4:19

GoogleCodeExporter commented 9 years ago
Oops, looks like the topic we are hijacking needed a status update :)

Original comment by HuguesLe...@gmail.com on 5 Jul 2013 at 4:20

GoogleCodeExporter commented 9 years ago

Original comment by HuguesLe...@gmail.com on 27 Jun 2014 at 7:50