Fix/finish EMBI, add TGIN internal support, fix EXTN potential crash #76

colinator27 · 2018-11-15T21:09:13Z

Exactly as the title says. This should fix a whole lot, including an oversight with EXTN I made. Apparently "product ID" only happens on newer games... (probably 1.4.9999 and 2+)

krzys-h · 2018-11-15T21:53:05Z

UndertaleModLib/Models/UndertaleTextureGroupInfo.cs

+        public void Serialize(UndertaleWriter writer)
+        {
+            writer.WriteUndertaleString(GroupName);
+            uint pointer1 = writer.Position; writer.Write(0);


Stuff like this was kinda supposed to be abstracted away into UndertaleReader/Writer to not clutter up code with manual pointer magic like this... (I also have to kinda fix it a bit to better handle non-standard pointers e.g. messed up by other tools, but that's another story). You can use WriteUndertaleObjectPointer as long as you make the target implement UndertaleObject (also see my note about lists which should handle that)

krzys-h · 2018-11-15T21:55:05Z

UndertaleModLib/Models/UndertaleTextureGroupInfo.cs

+            uint pointer4 = writer.Position; writer.Write(0);
+            uint pointer5 = writer.Position; writer.Write(0);
+
+            SeekWritePointer(writer, pointer1);


This is quite repetitive. How about implementing something like UndertaleSimpleList that handles UndertaleResourceById instead of normal objects? UndertaleSimpleResourcesList? Could probably be used in a couple more places as well, this is not the first time we see a sequence like that.

krzys-h · 2018-11-15T22:14:12Z

Ugh, you may run into some problems with that since ResourceById doesn't have a default constructor. I'll try to figure it out tomorrow.

colinator27 · 2018-11-15T23:24:41Z

Yeah, I was worried that was not done the greatest. I tried a bunch of different approaches but I also ran into some problems with ResourceById.

krzys-h · 2018-11-16T16:54:00Z

See 3951aca and 5b036f3, they should help with a cleaner implementation. You will need to update the code as the UndertaleResourceById syntax has changed.

You should now be able to simply do:

SomeList = reader.ReadUndertaleObjectPointer<UndertaleSimpleResourcesList<SomeObjectType, UndertaleChunkXXXX>>();
//...
var returned = reader.ReadUndertaleObject<UndertaleSimpleResourcesList<SomeObjectType, UndertaleChunkXXXX>>();
if (returned != SomeList) throw new Exception("I need to make it check this automatically somehow but for now do it like this (UndertaleRoom works like that too)");

colinator27 · 2018-11-16T19:37:21Z

Alright, thanks. That should be pretty helpful.

…variables

colinator27 · 2018-11-17T14:23:40Z

So now TGIN serialization is much better looking, thanks to that new class. However there are a few issues that may have to get fixed, for the codebase in general, about how reading of objects are done. Basically, if an object is never referred to as a pointer, then it should probably never be added to the object pool (or have anything to do with it). The reason why is because otherwise if you have an object at the start of another object, they can "overlap", and that causes errors when it tries to convert between the two types. This is easily fixed by manually initializing objects and then calling Serialize, but maybe there's a better approach?

By the way- the other changes:

I made sure the new chunks were added to the lists so that they were accessible.
I fixed FONT (look at New value in FONT (bytecode 17) #79) for bytecode version 17, as well as for some versions before (I have good reason to believe some changes start from 1.4.9999 from changelogs, but not 100% sure).
I named some variables for EmbeddedTexture, as well as slightly fixing it.
I created a new list, UndertaleSimpleListShort, which is the same as UndertaleSimpleList except that the count uses a 16-bit integer instead of a 32-bit integer.
I made sure bytecode 17 can load, and from what I've seen it's able to export mostly fine? I haven't seen any issues yet.

…kely what the values actually are

colinator27 · 2018-11-17T14:26:57Z

(just squeezed that in to satisfy the last couple comments of #70)

colinator27 · 2018-11-17T16:11:24Z

Okay, so hold on a little bit. I may have just discovered a major problem...

colinator27 · 2018-11-17T16:17:41Z

Phew, that's fixed now... It should be fine.

krzys-h · 2018-11-17T21:34:35Z

However there are a few issues that may have to get fixed, for the codebase in general, about how reading of objects are done. Basically, if an object is never referred to as a pointer, then it should probably never be added to the object pool (or have anything to do with it). The reason why is because otherwise if you have an object at the start of another object, they can "overlap", and that causes errors when it tries to convert between the two types.

Yeah, I know, I hit that limitation a couple of times. Having them all in the object map can be useful though, for example it's used for generating the offset map file that lists every single object read, which can aid in comparing hexdumps like we did for the two Switch versions that were almost identical. I'm still thinking of better ways to solve this, one may be to have a separate object map per object type.

krzys-h · 2018-11-17T21:37:51Z

UndertaleModLib/Models/UndertaleFont.cs

+                    Kerning = reader.ReadUndertaleObject<UndertaleSimpleListShort<GlyphKerning>>();
+                } else
+                {
+                    Offset = reader.ReadInt32(); // Maybe? I don't really know, but this definitely used to work


Maybe it was two bytes of data and two bytes of padding, or maybe the list always existed but was empty?

I guess maybe it can assume there's always a list? No real harm in doing so.

Hard to tell when we don't even have any examples that make use of it

UndertaleModLib/Models/UndertaleFont.cs

UndertaleModLib/Models/UndertaleRoom.cs

UndertaleModLib/UndertaleData.cs

krzys-h · 2018-11-17T21:46:32Z

UndertaleModLib/Models/UndertaleFont.cs

+                }
+            }
+
+            public class GlyphKerning : UndertaleObject


So I guess the editor needs an update...
How many entries of this do you usually get? I'm just wondering what controls would be best for this, since now every table row can have subrows...
(and font editing for translators just got harder, ugh)

Actually, I feel stupid because I just realized none of the things I've tested on use the list at all. It doesn't really seem necessary to allocate too much space for them (if any).

krzys-h · 2018-11-17T22:15:23Z

Also, please let me know if I missed anything we already know about bytecode 17 that should be listed on this page: https://github.com/krzys-h/UndertaleModTool/wiki/Bytecode-17

I should probably document other recent changes like the Spine sprites, the new font things and updates do GeneralInfo and Options flags, but I guess none of these are really related to the new bytecode right?

Ugh, we should seriously start writing proper documentation of the format like https://pcy.ulyssis.be/undertale/unpacking-corrected (but way more detailed, probably) instead of all these short patch documents

Co-Authored-By: colinator27 <[email protected]>

Sorry for making these several commits, but I'm far too lazy to do it the proper way. Co-Authored-By: colinator27 <[email protected]>

Co-Authored-By: colinator27 <[email protected]>

krzys-h · 2018-11-18T11:19:28Z

I was about to say that this suggestion feature really needs a way to accept multiple suggestions in one commit but...

To apply the change in its own commit, click Commit suggestion.

To add the suggestion to a batch of changes, click Add suggestion to batch. Continue to add the suggested changes you want to include in a single commit. When you've finished adding suggested changes, click Commit suggestions.

colinator27 · 2018-11-18T15:01:34Z

Wait, what?? I saw that button but it was grayed out...

krzys-h · 2018-11-18T15:06:32Z

No idea, this was the first time I used that feature, I don't even know how it looks on your side :P

Kneesnap · 2019-07-15T02:19:03Z

Is this resolved?

Kneesnap · 2019-07-15T02:19:12Z

If so, it should be closed.

Fix EMBI, add TGIN internal support, fix EXTN potential crash

8b0a235

This was referenced Nov 15, 2018

TGIN chunk (GM2) #70

Closed

Implement reading EMBI #74

Closed

Add editor for the EXTN chunk #4

Closed

krzys-h requested changes Nov 15, 2018

View reviewed changes

krzys-h added data.win format Working on the details of the data.win file enhancement New feature or request labels Nov 16, 2018

colinator27 added 2 commits November 16, 2018 14:38

Merge branch 'master' of https://github.com/krzys-h/UndertaleModTool

fb52558

Fixes to bytecode 17 changes, fix a few minor issues, name some more …

c6b10af

…variables

Change error messages to be about version numbers, because that is li…

427b1a1

…kely what the values actually are

Fixed one line that broke everything

544eec3

colinator27 mentioned this pull request Nov 17, 2018

REQUEST - Add TTFFonts script but for DeltaRune #40

Closed

Fix pre-padded object unserialization in pointer lists

b63a470

krzys-h mentioned this pull request Nov 17, 2018

Clean up the object reading code #80

Closed

krzys-h requested changes Nov 17, 2018

View reviewed changes

krzys-h and others added 4 commits November 17, 2018 17:23

Get rid of unnecessary GMS2 check #1

1f28c8e

Co-Authored-By: colinator27 <[email protected]>

Get rid of unnecessary GMS2 check #2

481bcb4

Co-Authored-By: colinator27 <[email protected]>

Remove unnecessary chunk from default list #1

86ba322

Sorry for making these several commits, but I'm far too lazy to do it the proper way. Co-Authored-By: colinator27 <[email protected]>

Remove unnecessary chunk from default list #2

0653028

Co-Authored-By: colinator27 <[email protected]>

krzys-h approved these changes Nov 18, 2018

View reviewed changes

krzys-h merged commit 424c73e into UnderminersTeam:master Nov 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/finish EMBI, add TGIN internal support, fix EXTN potential crash #76

Fix/finish EMBI, add TGIN internal support, fix EXTN potential crash #76

colinator27 commented Nov 15, 2018

krzys-h Nov 15, 2018

krzys-h Nov 15, 2018

krzys-h commented Nov 15, 2018

colinator27 commented Nov 15, 2018

krzys-h commented Nov 16, 2018

colinator27 commented Nov 16, 2018

colinator27 commented Nov 17, 2018

colinator27 commented Nov 17, 2018

colinator27 commented Nov 17, 2018

colinator27 commented Nov 17, 2018

krzys-h commented Nov 17, 2018

krzys-h Nov 17, 2018

colinator27 Nov 18, 2018

krzys-h Nov 18, 2018

krzys-h Nov 17, 2018

colinator27 Nov 17, 2018

krzys-h commented Nov 17, 2018

krzys-h commented Nov 18, 2018

colinator27 commented Nov 18, 2018

krzys-h commented Nov 18, 2018

Kneesnap commented Jul 15, 2019

Kneesnap commented Jul 15, 2019

Fix/finish EMBI, add TGIN internal support, fix EXTN potential crash #76

Fix/finish EMBI, add TGIN internal support, fix EXTN potential crash #76

Conversation

colinator27 commented Nov 15, 2018

krzys-h Nov 15, 2018

Choose a reason for hiding this comment

krzys-h Nov 15, 2018

Choose a reason for hiding this comment

krzys-h commented Nov 15, 2018

colinator27 commented Nov 15, 2018

krzys-h commented Nov 16, 2018

colinator27 commented Nov 16, 2018

colinator27 commented Nov 17, 2018

colinator27 commented Nov 17, 2018

colinator27 commented Nov 17, 2018

colinator27 commented Nov 17, 2018

krzys-h commented Nov 17, 2018

krzys-h Nov 17, 2018

Choose a reason for hiding this comment

colinator27 Nov 18, 2018

Choose a reason for hiding this comment

krzys-h Nov 18, 2018

Choose a reason for hiding this comment

krzys-h Nov 17, 2018

Choose a reason for hiding this comment

colinator27 Nov 17, 2018

Choose a reason for hiding this comment

krzys-h commented Nov 17, 2018

krzys-h commented Nov 18, 2018

colinator27 commented Nov 18, 2018

krzys-h commented Nov 18, 2018

Kneesnap commented Jul 15, 2019

Kneesnap commented Jul 15, 2019