Deleting characters

[AnywayFarus]

About The Pull Request

Adds a button to delete characters, and when deleted it flips to the nearest character, wow(the last character can't be deleted(I guess)).

Video:

NVIDIA_Share_19.06.12.15.442.mp4

Why It's Good For The Game

Once you know how to create characters, you sometimes want to be able to delete them.
The reasons for this can be... as simple as freeing up a slot, or deleting them in order to remake them.
I think this is quite a useful feature that should have been there before.

Changelog

🆑 Vishenka0704
qol: Ability to delete characters(yourself)
/:cl:

[MrMelbert]

It's kind of a weird place to put a delete character button, leaves a bunch of empty space

The desert

At the very least I would keep the button always-shown and just disable it when it's unusable.

[MrMelbert]

@Mothblocks Got an opinion on where it should go?

[optimumtact]

Does this button have a safety feature?

it should make you type the character name.

edit: nvm should have watched the video

[AnywayFarus]

It's kind of a weird place to put a delete character button, leaves a bunch of empty space

The desert

At the very least I would keep the button always-shown and just disable it when it's unusable.

[LemonInTheDark]

Your justification is bad. Why should we maintain this if we don't want it. We are not a library.
Also why does it not update the ui on delete in your video.

[LemonInTheDark]

see above

[ZephyrTFA]

if your justification is the downstream wants it.
publish it downstream

[AnywayFarus]

if your justification is the downstream wants it. publish it downstream

But no one above, except you two, has said you don't need this feature.
I came to you, not because you have to keep maintaining it, but in case you need it, that's how opensource works, no?

[AnywayFarus]

Your justification is bad. Why should we maintain this if we don't want it. We are not a library. Also why does it not update the ui on delete in your video.

Well, as you coded, that's how it works :).
Manual update used to break styles (and rarely even the whole pref-menu).

[mc-oofert]

I use 1 slot

[ZephyrTFA]

if your justification is the downstream wants it. publish it downstream

But no one above, except you two, has said you don't need this feature.
I came to you, not because you have to keep maintaining it, but in case you need it, that's how opensource works, no?

Your WIGFTG clearly says you are adding this for downstreams.

[AnywayFarus]

if your justification is the downstream wants it. publish it downstream

But no one above, except you two, has said you don't need this feature.
I came to you, not because you have to keep maintaining it, but in case you need it, that's how opensource works, no?

Your WIGFTG clearly says you are adding this for downstreams.

All the features that I've added here are necessary for me specifically, only on downstreams.
But I'm sharing them on the premise that you might find it useful.

[vinylspiders]

Your justification is bad. Why should we maintain this if we don't want it. We are not a library. Also why does it not update the ui on delete in your video.

I don't really see why this is relevant when being able to clear a character slot in-game without the need for manual savefile editing is something that is obviously useful to TG as well. It just kinda goes without saying?

I don't like that we are setting precedents here where I feel the need to warn folks against using the magic word 'downstream' whenever they try to make a PR here.

They are selling their idea extremely poorly, sure, but I mean this feels a bit silly to me as for a reason to be closing the PR on them (and just for the record I had nothing to do with this PR--this is just not the first time I've seen the 'we are not a library' argument being used, in my opinion, inappropriately).

[LemonInTheDark]

But no one above, except you two, has said you don't need this feature. I came to you, not because you have to keep maintaining it, but in case you need it, that's how opensource works, no?

I'm not looking to close this pr, I just want you to justify it in a way that is relevant to this codebase. I do not think the idea is inherently bad but your justification read as "This likely isn't useful to you but it is for me". I'm not sure if that was intended though.

You need some reason for us to potentially want this. Since you don't play here the reason you want it for downstream would work, it's possible that would fit some aspect of our project.

[LemonInTheDark]

if your justification is the downstream wants it. publish it downstream

I do not think this was justification to close, just justification to block since it's something we can discuss and resolve.

I don't like that we are setting precedents here where I feel the need to warn folks against using the magic word 'downstream' whenever they try to make a PR here.

If someone makes a pr here (or an issue report) it needs to be in the context of this project. I get many people come from downstream but if they're justifying something they need to justify it for THIS codebase. That's it. Can mention a downsteam while also doing that.

[vinylspiders]

if your justification is the downstream wants it. publish it downstream

I do not think this was justification to close, just justification to block since it's something we can discuss and resolve.

Gotcha--I have no qualms with that! But it was closed (just not by you) which led to what I see as a bit of a miscommunication and confusion, as well as frustration since they had already gotten a couple reviews from other maints.

If someone makes a pr here (or an issue report) it needs to be in the context of this project. I get many people come from downstream but if they're justifying something they need to justify it for THIS codebase. That's it. Can mention a downsteam while also doing that.

Fair. I just saw that it was closed (again, not by you) on the grounds of insufficient justification which felt a bit... I don't want to say rude but it kind of was?

Just as there should be justification for opening the pr, there should be sufficient justification for closing it too. And something like this I think it's plainly clear why it'd be good for TG as well, even if it's not said. Yknow?

If it's a matter of the implementation not being good enough, or UX design not being in line with what you want, that's valid. But that's not what the given reason was by the person who closed it.

I don't think we should be treating our contributors this way, but that's just my way of doing things I guess. If I were this person I might not be so eager to open a PR on TG ever again.

[LemonInTheDark]

Spoke to zypher, reopening conditional on some new justification

[vinylspiders]

Spoke to zypher, reopening conditional on some new justification

Cool. I don't mean to be a pain about this, but it's just something I noticed and felt the need to speak up about because it seemed a bit over the top. And hate to see another potential regular contrib be scared off over something silly

[LemonInTheDark]

Cool. I don't mean to be a pain about this, but it's just something I noticed and felt the need to speak up about because it seemed a bit over the top. And hate to see another potential regular contrib be scared off over something silly

If it makes you feel better it was not because of what you said. Reasonable concern to have tho.

[san7890]

to me, the value is apparent since i sometimes like the idea of having a new clean "new character" in that top bar and don't like it when i accidentally click into it and "spoil" it with an autogenerated name that needs to live on my character file forever now. i understand the pr author needs to make those changes/justifications but i still have interest in this PR - would have merged it already if it weren't for some concerns on the UI layout

[AnywayFarus]

Uh-oh-oh.
I've gotten better results already on the downstream itself, I'll try to transfer a similar thing here as well.
It's just a different format. of the character list, you have buttons and they have a drop-down list.
I found a solution for auto-updating... which kind of improves the interaction itself a hundredfold, and I think it would fit in here too.

However, I haven't figured out a better place for the TG than the button at the bottom.

[AnywayFarus]

And in general, after translation (or maybe in your language originally) it looked quite rude, like, I offered you qol, instead of saying that "thank you, but we need", they say "why the hell are you trying to put it in our hands".

[AnywayFarus]

Updated the functionality itself, updated the video with the current functionality, updated WIGFTG