Norbyte / lslib

Tools for manipulating Divinity Original Sin and Baldur's Gate 3 files
MIT License
765 stars 141 forks source link

ConverterApp fork changes #2

Closed fireundubh closed 5 years ago

fireundubh commented 7 years ago

So, I've been refactoring and adding new features to the ConverterApp all Sunday. I haven't committed all my changes to my fork yet, and it'll take me awhile to commit everything properly.

Not sure if you'll want to put in the time to pull anything, but here's a list of some changes:

I started out implementing the first two, but my efforts just escalated from there...

Any requests? 😄

Norbyte commented 7 years ago

Enhancements are always welcome! There are a lot of things that need a cleanup, but I didn't have any free time recently, so I was mostly bugfixing things when needed. The changes you've listed sound great, I'd definitely pull them 😄 The only thing I'm not completely sure about is the MessageBox.Show --> Status Bar change in the case of errors, but I'd have to see how it actually looks like to be able to judge if it actually fits.

No requests, though I guess I could share what potential future developments/fixes I had in mind:

I'm sure I had many more ideas for the lib but I don't remember any of them atm 😄

fireundubh commented 7 years ago

I'm not completely sure about is the MessageBox.Show --> Status Bar change in the case of errors

Agreed. I was thinking about that earlier. Showing a prompt for exceptions is a good idea. But the dialog should display an error code instead of the full call stack. Most users won't know what to do with that information. Maybe write the call stack to a file, and link the file in the dialog message. Or maybe forget about file logging and just create a form with a textarea, so the user can copy the call stack directly ("Copy to clipboard" button) or choose to save the call stack to a file ("Save..." button). Yeah, that sounds good.

I was also thinking about file associations. Associate .pak with the converter, for example. Double-click the .pak file, and the converter opens to the PAK tab with the input field populated with the path to the .pak file. Unless this is already done (I didn't look yet), I'll probably implement this as well.

Import/export/conversion

Can't really help with those systems, unfortunately. RE isn't my domain.

But I can refactor, optimize, and further develop the UI. I've also done a metric ton of CLI programming in a dozen languages, so I can build out a CLI for the converter as well.

Story extensions

Are the scripts compiled to bytecode? You could write your own compiler in that case. Guy named Orvid wrote Caprica for Fallout 4 scripts, for example. Caprica extends the Papyrus scripting language. Or maybe I'm misinterpreting "additional functions" as language extensions...

LS* resources

I'd love to be able to convert TXT to SQL or XLSX. I started writing a converter in Python awhile ago. Got as far as text to objects to JSON before the editor came out. Probably should look into NoSQL at some point. The driving idea was to export item recipes with full names, descriptions, stats, and icons to facilitate posting the recipes online. I can't believe some fans are doing that manually!

Norbyte commented 7 years ago

But the dialog should display an error code instead of the full call stack. Most users won't know what to do with that information. Maybe write the call stack to a file, and link the file in the dialog message. Or maybe forget about file logging and just create a form with a textarea, so the user can copy the call stack directly ("Copy to clipboard" button) or choose to save the call stack to a file ("Save..." button). Yeah, that sounds good.

My experience is that people will post "stuff is not working!" 50% of the time without posting any actual error message and the remaining 50% will send a screenshot of the error (with the call stack in it). I'm not a fan of error codes as they are basically a non-descriptive artificial construct that serves almost no purpose in this context. Error codes are good when the error conditions are well defined. They are useful for API-s, but are useless for actual user interactions. A good stack trace is worth more than a thousand words 😄. A copy feature would actually be useful though (although a not very well known feature of Windows message boxes is that pressing Ctrl+C will actually copy their contents!)

I was also thinking about file associations. Associate .pak with the converter, for example. Double-click the .pak file, and the converter opens to the PAK tab with the input field populated with the path to the .pak file. Unless this is already done (I didn't look yet), I'll probably implement this as well.

There are 2 issues I can see here:

Are the scripts compiled to bytecode? You could write your own compiler in that case.

This is not something that I've decided on so far; although I think I'd rather integrate a fairly well known language (eg. Python or Lua) than write a new one from scratch (which takes a veeery long time). The problem with Osiris is that it internally compiles to a ruleset, not bytecode. It cannot really be used for code that is complex or heavily branching (well technicall it can, but you're looking at dozens of indirect calls and queries to accomplish the same thing). There are two possible approaches though:

Guy named Orvid wrote Caprica for Fallout 4 scripts, for example. Caprica extends the Papyrus scripting language.

Eh, I've known him for his Facebook/HHVM work, didn't know he also wrote a Papyrus extender. His code does have that Facebook-y feel though :)

I'd love to be able to convert TXT to SQL or XLSX. I started writing a converter in Python awhile ago. Got as far as text to objects to JSON before the editor came out. Probably should look into NoSQL at some point. The driving idea was to export item recipes with full names, descriptions, stats, and icons to facilitate posting the recipes online. I can't believe some fans are doing that manually!

Yeah, the new TXT-s (that are JSON in disguise) are actually quite convertible to SQL. We could even connect an RDBMS and do realtime data sync between the DB and the JSON which would allow all kinds of nifty operations to be done on the data.

fireundubh commented 7 years ago

".pak" is a fairly common file extension for packages and is used by many apps; adding an association will also add an association for these other .pak files as well

Well, I'd assume the loader would verify the file magic before running the converter or loading the file info into the app. Nothing much to be done about overriding other .pak associations. User's call. There's also .gr2, .osi, .lsv, .lsx, .lsb, .lsf, and .lsj. Didn't mean to imply only associating .pak files.

people update their tools quite often, and many times they download/extract them to different locations so the .exe path in the file association would become invalid

So... I guess that calls for an installer? 😆 I fixed some bugs and added Fallout 4 support to the NSIS installer for Wrye Bash. It's doable, in terms of what I can contribute.

fireundubh commented 7 years ago

I unintentionally deleted my stashes when I reforked, so I started rebuilding my changes today.

I just committed persistent user settings and CreateDirectory error handling in ConverterApp and LSLib, but I don't know if pull requests are the right way to bring over changes to main. I've also done a lot of ReSharper refactoring, mostly in ConverterApp and Package* in LSLib, so conflicts are inevitable.

Some other things of note:

You can see what I've committed so far here: https://github.com/fireundubh/lslib/commits/master

fireundubh commented 7 years ago

Also, do you have a preference for CLI parser libraries? I was thinking of using Fluent.

Norbyte commented 7 years ago

I think pull requests are fine, they'll move the commit as is, instead of creating duplicate commits that are near-identical.

Commented out btnDebugExport_Click because sev.Visit(Story) isn't valid here.

It should be valid, it works just fine for me. Visit() has many overloads, but one of them is Visit(Story).

Renamed FileInfo in PackageCommon.cs because FileInfo shadows System.IO.FileInfo.

That's a good idea.

Set GR2 tab in/out formats with Path.GetExtension instead of string.Substring

Yess, that was a bit messy.

Also, do you have a preference for CLI parser libraries? I was thinking of using Fluent.

Fluent is OK!

Norbyte commented 7 years ago

Although I'd say we should pull things that are already done, so my changes won't as many conflicts.