Mephiles / torntools_extension

A browser extension for Torn.com
GNU General Public License v3.0
110 stars 62 forks source link

Option to disable Leave / Mug / Hospitalize on loader, like disabling gyms #600

Closed pomatias closed 2 years ago

pomatias commented 2 years ago

I love the way that TornTools allows me to disable certain training options in the gyms, to prevent me from accidentally clicking on them when I didn't mean to.

Since mis-click protection seems to be an okay thing to build, I would really like to have it for the leave/mug/hosp options as well:

If this is a feature that you'd like to add and somebody familiar with the codebase is willing to describe how they'd like it to be implemented ("do the checkboxes like they're done in this file, make it an option whether to show them like it's done over here, put it all [in a new file here | in with the other settings there]"), I'd be happy to take a shot at implementing it.

Sashank999 commented 2 years ago

We usually have these discussions in Discord on TornTools Server. But Github isn't a bad place. Mephiles and DKK decide on what features should be implemented while DKK and I implement them. Mephiles maintains server side things. Allo and Yukio(newly added to the group) are consulted for design choices and feedback on visual design.

We use Prettier for formatting. We discourage the use of inline HTML and inline CSS unless absolutely necessary or when HTML is too complex. If using inline HTML, format it manually. Prettier doesn't do that. Do not use 3rd party JS-simplification libraries like jQuery, unless absolutely required. ChartJS and others that are used for different purposes are valid. Any 3rd party libraries should be in extension/vendor, in its own folder.

You would have to do 4 things for any feature:

  1. Define its settings. Settings for all features are in extension/scripts/global/globalData.js, in DEFAULT_STORAGE, inside DEFAULT_STORAGE.settings.pages. The page for this feature would be attack. Add a new key there and use DefaultSetting class.
  2. Add the JS and CSS files for the feature in extension/scripts/features, in a new folder.
    • You could use hide-attack-options, but you can name it anything.
    • Use all lower case letters and use - instead of spaces.
    • JS and CSS files should have the same name, but different extension. And files should have tt prefixed, no spaces and should follow camelCase.
    • Use featureManager for every feature. It handles the loading of every feature, shows its status in Feature Manager popup, disables feature during live reload, catches errors.
  3. Add the feature's setting in extension/pages/settings/settings.html, in line 1347.
    • The section name should match the page you defined in DEFAULT_STORAGE.
    • Every feature should have its own setting.
    • And be sure to have labels for checkboxes.
  4. Add the JS and CSS files in extension/manifest.json, for the corresponding page URL. Line 476 should be the right location.

Another general rule is to use JS and CSS that is compatible with the latest versions of Chrome, Firefox, Kiwi and Yandex Browsers. You would have issues with Kiwi on optional permissions but just ignore that.

DeKleineKobini commented 2 years ago

I don't think this is something that the broader user base would want / need / use.

During chains it would be valid to hosp enemies, as it gives the same respect. During warring, leaves are even preferred in some cases (like offline people, during a raid). And in fact, SA is mugging me regulary atm, during a war. During a mission, you might also attack others (as maybe the target is still in hosp).

And that is just the logic about it. Implementing it wouldn't be easy either. Missions aren't detectable using the api. Iirc, for wars/chains you need faction api access.

Sashank999 commented 2 years ago

This feature has a respectable user base. You don't need to have missions to be detected. There are a lot of factions having friendly wars and they would go on for like 2 days at once. They need this feature.

DeKleineKobini commented 2 years ago

This feature has a respectable user base.

I don't agree. It will have to be off by default, instantly reducing the potential user base a huge amount. When it's activated, I can see players disabling is again due to the many exceptions to the rules, making it unusable.

You don't need to have missions to be detected. There are a lot of factions having friendly wars and they would go on for like 2 days at once. They need this feature.

Sure, they need it that hard to the point that this is the feature suggestion of this type. Most users won't even know it's there due to it being off by default.

DeKleineKobini commented 2 years ago

I've also opened an internal discussion about this, with the final word to Mephiles.

DeKleineKobini commented 2 years ago

We've finished discussing it internally.

It's been accepted as suggestion. There is only 1 requirement, and that is it being possible to be disabled (checkboxes not being shown).

@Sashank999 is willing to do it, but is also open to let you make it. That's something I'll leave up to you @pomatias.

pomatias commented 2 years ago

Thank you for shepherding that discussion through! I'll bet @Sashank999 would get it done quicker and better than I would, meaning less headache for their reviewers. How about we plan on them doing it, for now?

If I come back to this issue in a few months and there hasn't been time for better coders to have a go at it, of course, I might try putting together a PR. But for now, I don't anticipate having a whole lot of time to learn browser extension development at the moment, so I'd be very grateful if @Sashank999 could handle it.

Thank you so much!