Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Game Path should be removed from the setup GUI #54

Open
Cokemonkey11 opened this issue Dec 5, 2020 · 4 comments
Open

Game Path should be removed from the setup GUI #54

Cokemonkey11 opened this issue Dec 5, 2020 · 4 comments

Comments

@Cokemonkey11
Copy link
Contributor

As discussed in #51, instead of revamping the feature for detecting wc3 game path, we should instead delete the feature from wurstsetup.

This has some benefits:

  1. Consolidates the set of places that a wurst user encounters the idea of a game path
  2. Reduces support burden as blizzard keeps fucking the detectable game path
  3. Simplifies the workflow for creating and updating projects

I am happy to do this work if I get a +1 from a maintainer @peq @Frotty .

My approach to deleting this feature is just find gamePathTF (only present in MainWindow.kt), deleting it, deleting dead code that comes out of doing so, and then testing to verify that it had the expected effect.

  • Double-check also that the text "Game path" is eradicated
  • Double-check that the [...] icon and its click handler are also properly deleted
@Frotty
Copy link
Member

Frotty commented Dec 6, 2020

Thanks for creating the issue. Perhaps removing it from setup and then adding handling to the compiler to open a file browser if wc3 dir cannot be detected would be best.
Autodetection finds default install locations, but if it's located in a custom directory (because of previous classic installation e.g.) right now the run commands simply fail without good user feedback.

@Cokemonkey11
Copy link
Contributor Author

@Frotty thanks for response.

As above, I'm happy to do the work to remove this from setup. However, your comments above made me think that perhaps the compiler feature is needed first to keep the feeling of "no feature is lost".

To be clear: will you approve a PR that removes this from setup, even if I don't have a follow-up for compiler?

Cheers

@Frotty
Copy link
Member

Frotty commented Dec 8, 2020

To be clear: will you approve a PR that removes this from setup, even if I don't have a follow-up for compiler?

No cuz then there is no gui possibility to change the path

@Cokemonkey11
Copy link
Contributor Author

@Frotty okay - I don't plan to make this contribution on compiler. Shall we reopen #51 instead to do some basic maintenance on the existing feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants