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

Disable OW followers by default #4720

Merged
merged 2 commits into from
Jul 11, 2024

Conversation

Bassoonian
Copy link
Collaborator

Disables OW followers by default, preventing us from silently activating them in projects that may not want it. For precedent regarding including features but having them off by default, see the VS Seeker and HGSS Dex implementations.

Discord contact info

bassoonian

@Bassoonian Bassoonian added this to the 1.9.0 milestone Jun 4, 2024
@AlexOn1ine
Copy link
Collaborator

We should consider turning all features on in debug mode. See #4664

@Bassoonian
Copy link
Collaborator Author

We should consider turning all features on in debug mode. See #4664

I think that this is out of scope for the default, for one, so re-enabling followers in debug mode should be in that PR rather than this one. I also think that a debug build for expansion can very well differ from a user's debug build (which would presumably want to preserve their selection of features rather than all), but that's a discussion for that PR

@AsparagusEduardo
Copy link
Collaborator

Disables OW followers by default, preventing us from silently activating them in projects that may not want it. For precedent regarding including features but having them off by default, see the VS Seeker and HGSS Dex implementations.

I actually believe that we should start doing the opposite for HGSS Dex, as a way to showcase features before they decide to disable them, plus making it easier on us to debug issues with it (for this last point, I need to add an option to the debug menu to see the vanilla dex when the HGSS menu is enabled)
We already allow them to opt out of undesired features, so having new features enabled by default for new users should be fine.

@AlexOn1ine
Copy link
Collaborator

AlexOn1ine commented Jun 4, 2024

Disables OW followers by default, preventing us from silently activating them in projects that may not want it. For precedent regarding including features but having them off by default, see the VS Seeker and HGSS Dex implementations.

I actually believe that we should start doing the opposite for HGSS Dex, as a way to showcase features before they decide to disable them, plus making it easier on us to debug issues with it (for this last point, I need to add an option to the debug menu to see the vanilla dex when the HGSS menu is enabled) We already allow them to opt out of undesired features, so having new features enabled by default for new users should be fine.

Then we should disable all species past gen3 otherwise we will run into space issues sooner then later.

@AsparagusEduardo
Copy link
Collaborator

Then we should disable all species past gen3 otherwise we will run into space issues sooner then later.

That's a different discussion altogether. Features like HGSS Dex or potential future DexNav will never compare to the size used by Pokémon cries.
Heck, adding follower sprites for Gens 1-8 only took 3.4% of the total size, while cries take around 25%!

@AlexOn1ine
Copy link
Collaborator

Then we should disable all species past gen3 otherwise we will run into space issues sooner then later.

That's a different discussion altogether. Features like HGSS Dex or potential future DexNav will never compare to the size used by Pokémon cries. Heck, adding follower sprites for Gens 1-8 only took 3.4% of the total size, while cries take around 25%!

Yeah sorry I just checked. Somehow I thought it adds much more.

@AsparagusEduardo AsparagusEduardo marked this pull request as draft June 4, 2024 18:45
@DizzyEggg
Copy link
Collaborator

Should this be closed now that we've decided to keep new features enabled?

@Bassoonian Bassoonian marked this pull request as ready for review July 11, 2024 12:09
@Bassoonian
Copy link
Collaborator Author

Ready for review again, fixed conflicts and added an explicit warning about additional scripting being required as discussed on Discord.

@ghoulslash ghoulslash merged commit 7c23a39 into rh-hideout:upcoming Jul 11, 2024
1 check passed
@Bassoonian Bassoonian deleted the disablefollowersbydefault branch July 27, 2024 19:16
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

Successfully merging this pull request may close these issues.

5 participants