search

momentum-mod / lumper

BSP lump editor and review tool
MIT License
20 stars 13 forks source link

UI rework and major cleanup #39

Open tsa96 opened 3 weeks ago

tsa96 commented 3 weeks ago

Heavily reworks the UI to create dedicated pages:

I was always strongly against trying to represent the entire BSP in a tree view given how different and usually unrelated each lumps are. I have a few classes/interfaces for capturing common properties that different lump viewmodels share, but generally I've tried to keep them quite separate. I've also put a lot of effort into allow multiple pages edit the same underlying lump viewmodels, for example running a Stripper job (what a name, fucking hell) will update the entity lump and therefore any pages in the entity editor displaying it. This should making adding future pages quite easy, and hopefully fun to do.

Since we've been discussing this regularly in Discord I'm not going to bother summarize every other change. The new readme shows screenshots for all the new stuff if you don't want to build yourself, also I'll get a release build out via Github in the next few days.

Obviously this is a lot of work to review, please give me a shout if you'd like to run through stuff in voice with me if you think that'd save you time.

I'm going to add CI and maybe a couple other small features in the next couple days, but I'll add those commits on top of the current stuff.

Closes #5 Closes #14 Closes #27 Closes #21 Closes #15 Closes #7 Closes #2 Closes #38 Closes #44 Closes #45 Closes #46 (some of these were already done, but definitively closed by this)

tsa96 commented 2 weeks ago

Since I've now cut 0.1.0 release from this branch I'm going to avoid force-pushing from now on, so some commits may build off of existing commits, in particular in BspFile.cs, may find it easier to review all files at once, up to you.

tsa96 commented 3 days ago

more review today .. don't know how to comment on a specific commit and not file/change so I do it here: 7497214b4191fab5558207d7db04ccb6d19fa7fe commit message is bad. Title is too long (github cuts it off, don't remember the limit) and it says something about merging commits and there is no reference to them and thats not useful for anyone else to look at in the future. Also I don't think its necessary to point out the "Property" class renaming and when there are sooooo many changes like this.

The commit message is there to explain why the commit is doing two separate things at once, renaming variables and moving to primary ctors. When trying to split up that commit there was so much overlap that it was both harder to split up, and harder to understand.

Will reword that and some other long commits in next force-push.

tsa96 commented 3 days ago

Okay I think I've addressed most stuff here