juwonlee123 / tesv-snip

Automatically exported from code.google.com/p/tesv-snip
0 stars 0 forks source link

Warning: Subrecord doesn't seem to match the expected structure #10

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Open the file below
2. Go to STAT or ARMA
3. There is a message that says "Warning: Subrecord doesn't seem to match the 
expected structure"

What is the expected output?

I would like to use the routine that displays that message in another way.  I'd 
like there to be a check of the file when it is loaded.  If there are errors 
like Snip detects once I select a record, I'd like there to be a message window 
that pops up and tells you there are errors when the file is loaded.  Example:

"Warning: Some subrecords don't seem to match the expected structure.  Please 
load the plugin in the CK before using it with TESVSnip."

What do you see instead?

I'm sure this is expected.  The plugin is old and a lot has changed in the CK.  
The plugin is either corrupt or just old. I see "Warning: Subrecord doesn't 
seem to match the expected structure" when I click on STAT or ARMA entries.

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

Current Dev version

Please provide any additional information below.
    darksiders_beast_hood_pack.esp 
11.5 KB   Download  

Delete comment Report abuse Comment 1 by project member danielhmpdx16, Aug 26, 
2012
(No comment was entered for this change.)
Labels: -Priority-High Priority-VeryHigh
Delete comment Report abuse Comment 2 by project member leandor, Aug 27, 2012
I've checked the code, and saw that the message you quoted comes from the 
element parsing: is caused when an exception is raised during the conversion 
procedure when the XML structure requires more data than there is present on 
the record buffer.

Basically, when the plugin is read each element decodes to a byte array with a 
determined size, then, when the XML is used to interpret that buffer, the 
XML-controlled description requires more data than the buffer has.

I don't know why it happens still... I've been looking through the code but I 
couldn't find any changes that could explain why it fails there.

I've seen that it fails for the MODL and EDID sub-records, but the same EDID 
sub-record works correctly for the WEAP record. The only difference is that the 
EDID and MODL sub-record for the STAT record is declared on the XML file by 
using an intermediary Group.

I'll have to check this more thoroughly.
Delete comment Report abuse Comment 3 by project member danielhmpdx16, Aug 27, 
2012
I attached the file because when I load the file in the CK and save it then the 
error does not occur in the new file.
Delete comment Report abuse Comment 4 by project member danielhmpdx16, Aug 27, 
2012
Also the names are changed.  So this is a made up name but say I have 
(ArmorGodSteelAA.) as an ARMA record.  Notice there is a '.' at the end of the 
name.  After I load it in the CK it's called. (ArmorGodSteelAAdds)

This is why I feel that the forms are very old from pre 1.5 and maybe changes 
were made and loading it into the CK fixes the missing Data.  Which is why I 
want to use that message to check the records and tell you when some of them 
don't seem to match the XML.   I have not tried to dump it yet.  I'm gonna do 
that and look at the ARMA and STAT records.
Delete comment Report abuse Comment 5 by project member danielhmpdx16, Aug 27, 
2012
This is also why I added  issue43  so that I can more clearly see the form 
versions and possibly add the information to the UESP Wiki.
Delete comment Report abuse Comment 6 by project member leandor, Aug 27, 2012
Oh, Ok then. I'll have to check more if that behavior could be implemented at 
load time, but later, perhaps tomorrow.
Delete comment Report abuse Comment 7 by project member danielhmpdx16, Aug 27, 
2012
Whenever is fine thanks for looking at it.
Delete comment Report abuse Comment 8 by project member danielhmpdx16, Aug 27, 
2012
TES5Dump shows that the ARMA records are in fact out of order.  There is also a 
Footstep sound for some of them when they aren't even boots.  This is causing 
the error more than likely.  It's also removed by the CK when the plugin is 
saved.  When I looked at the new plugin only boots had a footstep sound, which 
makes sense.

So like I was thinking, it's pre 1.5 or corrupt.  In either case this is a good 
test plugin to get the error message to show up at load time as another way to 
help safeguard against mal formed plugins.  I think anything that can help the 
program be informative like this will make users feel more comfortable using 
Snip.
Delete comment Report abuse Comment 9 by project member leandor, Aug 28, 2012
Got that. Thanks for the plugin. I guessed that was the case when I looked at 
it in the Hex editor. Lot of strange things there :)

The trouble with this change (I'm referring to making these errors appear while 
loading) is that snip is designed different than xDump. It loads the files 
following the record/sub-record headers, but not caring about element/field 
content vs. xDump parses the file in its entirety validating the file's 
structure against the defined rules. xDump has much more knowledge about 
record's and field's contents than snip or any other editor, for that matter.

In Snip's case the XML structure file is only used for displaying purposes: to 
construct the field editor UI and to convert the field's raw content (according 
to their declared types) to an human friendly display string (what you see in 
the 'Report' pane).

It would require some perhaps not-so-difficult restructuring to work around 
that design philosophy. But restructuring is always hard to do when the program 
is so 'entangled' to begin with.

For now I think we can detect malformatted plugins at the bare/raw data level 
during the parse phase (that plugin has some examples of that), but at field 
level... Hmmm... I feel that is going a little against the design philosophy. I 
mean, you can edit a field by touching its bytes in an HEX editor!! :P

I guess we can add a check that tries to verify that field has the correct 
formatting for its declared type, but only validating the formal field 
definition without trying to validate the contents. For example, when a field 
is declared as string, it should have a null-terminator at end, and the field 
size should include it (I bring this example because that plugin has a lot of 
string fields that have no null-terminator at the end, and that confuses Snip's 
string field's reader, and gives those errors for the MODT and EDID field 
contents of the STAT records.)

I think that is a sort of middle ground that we can achieve more quickly than 
the full redesign of the parser that would be required to accomplish this thing 
like xDump does.

What do you think?
Delete comment Report abuse Comment 10 by project member danielhmpdx16, Aug 28, 
2012
>> null-terminator at end, and the field size should include it

Some strings in Bethesda's esm files don't. "I think."  Don't quote me on that. 
 I thought I saw Zilav mention this to me before when he added lstrings to 
Dump.  I may be thinking about the wrong thing though.

I was hoping to just use the existing routine to display an error message like 
I see when I select the record itself.  I see from your description that the 
error message does not display under the same circumstances I thought it did.

>> I think that is a sort of middle ground that we can achieve more quickly 
than the 
>> full redesign of the parser that would be required to accomplish this thing 
like 
>> xDump does.

I trust your judgement.  It seems like you have a way to accomplish this 
without a full redesign.  In fact, I like the fact that Snip does not work the 
same as Dump and does not care about element/field content so it would be nice 
if that did not change.  

I mainly don't want Snip to change so that we can add the functionality to edit 
the save games later on.  Snip's current design seems to allow that to happen 
by just adding information to an xml file and the displaying the information 
that is found in the save game.  I'm worried if too many changes occur this 
won't be easy.  Also there is the goal of adding Morrowind, and oblivion 
support.  

So yeah, lets not change things too much.  I'm glad you mentioned that.

>> to do when the program is so 'entangled' to begin with.

That makes sense.  Lets put this on normal priority and look at it again later 
after Snip has undergone some other changes first.  Especially changes that 
could allow this and other error message to be added easier.  

Since there has not been any peer review yet I'm thinking we will need other 
error messages.  Maybe it would be better to re-design the error messages to 
include this and any new error messages once more work has been done and we 
have had some peer review.  Then you won't be revising the same thing over and 
over.  It can just be done all at once.
Labels: -Priority-VeryHigh Priority-Normal
Delete comment Report abuse Comment 11 by project member danielhmpdx16, Aug 30, 
2012
'string'  is zero terminated.
'bstring' has a short (2byte) that leads that specifies the length.
'istring' has a int (4byte) that leads that specifies the length.
'lstring' is a localized string that can be either a int (4bytes that points to 
a string in a localized string file or a zero terminated string.
'str4'    is a 4 byte long string without any length or ending zero. Ie WEAP, 
ARMO
Delete comment Report abuse Comment 12 by project member danielhmpdx16, Aug 30, 
2012
Another thing I would have to check is what types of strings Dump uses.  I 
think Dump knows automatically which type it is.
Delete comment Report abuse Comment 13 by project member danielhmpdx16, Aug 30, 
2012
Also I would have to see what formats on the UESP Wiki are used by Dump and 
Snip.

http://www.uesp.net/wiki/Tes5Mod:File_Format_Conventions

Original issue reported on code.google.com by danielhmpdx16@gmail.com on 22 Sep 2012 at 11:54

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by danielhmpdx16@gmail.com on 23 Sep 2012 at 12:15