LadyDefile / Wordsmith-DalamudPlugin

Wordsmith is a Dalamud Plugin aiming to make roleplay easier, more convenient, and harder to accidentally "wrong chat" your post.
7 stars 3 forks source link

Scratchpad number always incrementing #30

Closed thespookyfish closed 1 year ago

thespookyfish commented 1 year ago

If a low number (say 1) scratchpad is deleted, there's no way to get another scratchpad 1. Typing /scratchpad 1 will open scratchpad 2. The number commands seem to only open the appropriate number scratchpad if that pad happens to exist, and will otherwise open a fresh one with a higher number. Admit there is a chance this is intended in some form, but I thought I'd report this, in the off chance that the intended behaviour is for it to recycle previously deleted numbers.

image

As seen in the above image, I cannot get scratchpads 1-3 back again.

LadyDefile commented 1 year ago

So, I was going to respond right away but I decided to take a moment to step back and see how difficult it would be to change this behavior. I saw your message almost immediately after you posted it (yay email notifications) and popped open my source code right away to do a little digging. Now that I've played around with a little, here is what I've got to say.

Firstly, thanks for posting about this. I love getting feedback. Knowing others are using Wordsmith and like it enough to write about features they want, bugs they noticed, etc. is a huge motivational boost to continue improving the plugin.

So, this behavior wasn't necessarily intentional but just a byproduct of the early Wordsmith code designs. The way that Scratch pad IDs are handled is integral to being able to open a (effectively) unlimited number of them. Every pad has a unique ID number that is used to identify it to the Dalamud plugin system (freaky things happen when two ImGui objects have the same name).

So, creating a new pad with an already disposed ID is simple. I was able to fix that with literally the 1/4 of a line of code, just changed one little thing and boom. I actually already had all of the code I needed in place because of all of the refactoring and rebuilding I've done up until now. Making the change was literally just changing a single line from "new ScratchPadUI(ScratchPadUI.GetNextAvailableID()); to new ScratchPadUI(id);

The issue that immediately popped up after that though was that creating a pad with an already used ID added it into the list in the wrong spot (i.e. {3, 4, 5, 6, 1, 7}) which made it display weird in the Settings UI. But, a quick little snippet of code to sort the displayed list in the settings and voila.

The only real concern I have now is that you are sorting this list every frame that SettingsUI is open. I'll do a little more digging around to find a more elegant solution to the sorting issue but you can count on the next update to Wordsmith allowing you to create old pads.

TLDR;

This behavior was a side effect of old code and was not intentional but not considered a bug. I've since implemented a change to give the desired behavior and it should be live with the next Wordsmith update.