EDCD / EDDI

Companion application for Elite Dangerous
Other
445 stars 81 forks source link

Speech Responder: Cannot disable/delete some responders. #325

Closed Valethar closed 5 years ago

Valethar commented 6 years ago

EDDI version in which issue found

[2.4.5]

Steps to reproduce

  1. Cannot turn off or successfully delete the following:

Commodity Purchase Check Commodity Sale Check

Attempting to disable the tick in the check box does nothing. It's permanently on. If I delete the command, it comes back again the next time the program starts.

Expected

Clicking the checkbox should disable the responder, as it does with most. Clicking the 'delete' button should remove it completely, especially when the button is not already greyed out, and you are given a dialog to confirm the deletion.

Observed

Check boxes nonfunctional on some commands. Delete process nonfunctional on some commands.

Investigation

Have not regressed to previous versions nor created yet another new 'personality'. It shouldn't require having to recreate the personality profile every time one wishes to activate or deactivate a responder.

Tkael commented 6 years ago

At present, only scripts which are directly triggered by an in-game event can be disabled. The scripts that you've noted here are not triggered directly by in-game events. Instead, they are "secondary"/"helper" scripts that are called by other scripts. We've made this distinction clearer in recent betas, but those changes haven't yet made it to regular users.

That said, this is an area of recurring confusion, particularly with new users. We need to investigate other options for handling this (such as allowing scripts to be disabled even if they are part of another script)

richardbuckle commented 6 years ago

We addressed this in public beta 2.4.6-b1 with enhancements to the explanatory text in the UI. Let's hold until the wider user base has seen that.

Valethar commented 6 years ago

Ah! That would explain it.

I've been using EDDI for quite a while now, but recently decided to turn those off, because it was getting a little too chatty while running missions.

I did a workaround for it, by editing the scripts and taking out all the text. I'm not happy with it, because I'm sure I'll probably break something by doing so, but it works for a temporary solution.

That also explains why it was still giving me some info on commodities, just not as much.

Is there a different primary script I should have been looking for instead? Despite using EDDI for the better part of a year, I've never really delved too deeply into the guts, so to speak.

Thanks for the assist. :D

Tkael commented 6 years ago

Try changing the ship role in the shipyard. These scripts are called from the 'Market information updated' script and only run if the ship's role is set to "Trading" or "Multi-purpose".

richardbuckle commented 6 years ago

Aha, I wasn't aware that the ship role had that effect! Maybe that needs a bit of publicity.

Tkael commented 6 years ago

We probably ought to customize more of the scripts according to ship roles... but how best to publicize? A section in the wiki?

richardbuckle commented 6 years ago

Am I right in thinking it's not amenable to auto-generation? Im thinking a page on the wiki with a section for each role, a link to it on the shipyard tab and a mention in the forum thread.

Tkael commented 6 years ago

I don't think it is amenable to auto generation. Maybe we just need better descriptions for the existing scripts?

richardbuckle commented 6 years ago

On reflection what is missing is descriptions of the ship roles rather than the individual scripts. That is to say, we don't have anything that helps the user to decide which role to assign to their ships when they are looking at the shipyard list.

Tkael commented 6 years ago

Right now, the roles have very little effect on the scripts. This is essentially the only instance where the role currently has an effect (of which I am aware). That said, we may wish to do a pass through the default scripts and assign some of the existing information to different roles too.

richardbuckle commented 6 years ago

I just did a search on the JSON and yes it's currently only the "Market information updated" event that uses the ship role in both the default personality and Darkcyde's.

Probably best to get this iteration squared away before embarking on another round of script development?

Djuwanna commented 6 years ago

I'd like to be able to disable some of these and others as well, such as the Fuel Check, the System State Report, some of the Galnet stuff (or all of it) - isn't is possible to permit us to uncheck the "Enabled" option for these, or to prevent them from adding themselves back in if deleted deliberately? I too am editing the content of the scripts to block the parts I don't want, and it's frustrating. I feel like I am forced to accept all-or-nothing as the only viable "approved" options (i.e. plugin enabled with options I don't want, or plugin disabled killing things I DO want), or to get creative with scripting (which I am sure you are much better at than I am).

Is it possible to enable the option to turn off these subroutines? Will it break the program if you give users that ability? If it will break it, how about considering a rescripting somewhere down the road (soon) to make sure it DOESN'T break it? I love the program, but there are times when it's just too chatty for me and I'd like to dumb certain things down without losing the other stuff I love about it.

Thoughts?

Djuwanna commented 6 years ago

(tap..tap...tap) Is this thing on? ;-)

Tkael commented 6 years ago

It's a fair question, and one we want to investigate further. But I've been deep in other areas of the code this week and haven't had time to look into this question in depth yet.

Djuwanna commented 6 years ago

Bless you for taking the time to respond (and doubly so for all you do!) Keep up the great work!! If you have time, one last quick question – eta on b2? I really want my VA responses back, and it looks like you squashed that one. Any chance it’ll be soon? Or maybe a b1.5 to get that part back up? No pressure lol, but you know how us CMDR’s are – gimme gimme gimme! Anyway thanks for all the hard work!!

richardbuckle commented 6 years ago

It'll be released as soon as it's ready, but not before. Won't be long now but the ship monitor is still not working right and I'm not willing to do a half-baked release. Please be patient just a while longer.

I'd be the first to agree that the Speech Responder UI is unfriendly and unintuitive. It's a classic example of what can happen when engineers "design" UI, or rather when they don't. All the nuts and bolts of the underlying mechanism are directly exposed, which means that technically one has the means to do anything, but the interface only makes sense to the people who wrote it.

So yes, the whole team is agreed that this part of the UI needs re-work, but right now we have more immediate concerns, and we want to take the time to do a proper job. Doing UI design "on the hoof" is how the existing UI came about and we don't want to repeat the mistake.

Djuwanna commented 6 years ago

That, sir, is the most respectful, complete, concise and honest answer I have ever received from a developer – and I thank you. I wish the guys that coded for me were as forthright!

Keep up the awesome work…I’ll stop pestering you and just keep checking in on the downloads page daily until it changes 😉

Go have a bourbon – I know I will!

o7

CMDR Djuwanna “For the mug!”

richardbuckle commented 5 years ago

Closing as it requires a UI rework and we should think about it afresh.