fix ui freeze on bsp load and add a progressbar

[ddorab]

It's been quite a while since I've used C# and it's my first time using Avalonia so excuse me if you see any bad code. I want to do a percentage based progress bar but I don't quite know how to properly style it out so I left it out for now, otherwise the calculation is pretty simple.

_lumps.Count / _parent.BspFile.Lumps.Count * 100

[tsa96]

@BIRD311 if you could check out as well that'd be great, feel free to merge whenever you're happy

[ddorab]

maybe not merge yet, I want to add the save freeze fix to this pr as well. Lmk if you want it to be a separate pr ofc. It'll be either tomorrow or the other day though, just got home and I'll go to bed soon.

[BIRD311]

your commit message is bad .. you want a short title in the first line, an empty line and then a longer description (didn't find any reference for this other than "that's what git commit -m "foo" -m "bar" does") .. if you don't leave the second line empty ,you get logs like this 873100b (HEAD -> pr31) fix: ui freeze on bsp load feat: add a progressbar

If you want to keep the fix and the progressbar seperate they should be two seperate commits, otherwise you should change the message .. I like the first option more

And I found a bug: when passing a map as the first argument it should get loaded right away, but it doesn't anymore

[ddorab]

I'm hoping this is good, I asked Tom how I should separate the commits and hopefully I understood him correctly

[tsa96]

Could maybe have separated out the stuff adding the progress bar but not a big deal

[ddorab]

okay :D excuse me I'm stupid, a little side note: I moved the progress bar to the center of the screen instead of being on the title bar because I'm planning on putting the name of file to where the progress bar was. Also I still haven't looked at the freeze on save but that'll be the 4th commit here I guess. At least I understand how you guys want the commits to look like now

[BIRD311]

• did some testing and it looks like it takes significantly longer now .. tested with surf_classics3 and it didn't finish saving after more than 10 minutes .. can you test this again please
• I'm still confused why your first commit breaks the "cli argument" and why we need the OnLoad to fix it
• not sure I like the 2 progress bars and I think we should make it so you can't change stuff or load new maps while its sill saving
• all the save functions just do Task.Run and run one after the other .. either try to get it more parallel or just do the Task.Run once outside

[ddorab]

• I explained the cli arguments thing in the commit message.
• Let me know how I should do the progressbar (or remove it)
• Yeah well the Task.Runs were just stupid, I asked in discord but I didn't get a response so I just did it this way.

[BIRD311]

it says "This fixes a bug introduced in my previous commit" but you shouldn't have to do this since its the same PR .. and, to be clear, I'm not saying you should squash those, I'm just asking why it broke in the first place and why it needs a "new" implementation

for everything else: I just gave feedback, pointed out bugs and questioned some decisions .. doesn't mean you have to change all of it, but you should be able to tell me why you decided to do it this why (like: what are we gaining from using 2 progress bars instead of 1) .. and I've answered the discord questions as good as I could without trying to implement it myself

[ddorab]

I've gone ahead and reverted the stupid save stuff I did before. It seems to take around the same time now. I've also updated the description on the cli arguments commit.

As for the progress bar, I've removed it but I do think it's useful. I'm also not a fan of having two but I couldn't think of anything better at the time. I think having a footer sort of like IDA if you're familiar with it will be nice. The reason I implemented the second one was that I had moved the first one to the center (it's still there btw). Let me know how you think the progress bar should be.

• Should I move just the one to the top and leave it like that, using it for both loading and saving?

• Should we just not have a progress bar at all for saving? (And maybe implement the footer just adding a "Saved" text or something along with other information that could be useful)

  Note: I could try and make the saving code a bit more parallelized like you suggested if you want, I just didn't think it was necessary since saving already takes little to no time.

[BIRD311]

I like the progress bar in the middle and hiding everything because you're not supposed to click around and change stuff while its loading anyway. For saving its basically the same and even if its fast, its nice to see that its doing something and when its done.

Looks like the loading progressbar only shows the first time and not if you load a different file later.

[ddorab]

Looks like the loading progressbar only shows the first time and not if you load a different file later.

I kept this out of the commits on purpose because I wanted to know what to do while saving first, but it's a really small change

[tsa96]

Closing, as my incoming PR reworks IO significantly and covers all of this.

My PR introduces a singleton service with sole responsibility for save/loading. FWIW, the IsModified AutoRefresh system wasn't just a developer nightmare, but it was also had a huge performance impact - we should never have been constructing that enormous observable chain. I still use reactiveui and observables heavily, but I've tried to keep the construction of observable networks to single places (constructors! Read the reactive recommendations!) and comment it all a lot.

I've also given some introductory reading recommendations for reactiveui on this repo's wiki, which is hopefully of help to anyone struggling with reactivity in the future.