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

Fixing variable scoping error #2143

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixing variable scoping error #2143

wants to merge 1 commit into from

Conversation

EvGym
Copy link
Contributor

@EvGym EvGym commented Aug 29, 2023

Apparently I cannot test locally. Noted for the future. Sorry about that chaps

@HoeenCoder
Copy link
Member

You will need to re-add your previous changes too as they were reverted.

I'll take the time to help fix your problems with testing when I am able too. Earliest would probably be tomorrow night assuming I still have power (hurricane).

@EvGym
Copy link
Contributor Author

EvGym commented Aug 29, 2023

Easy enough, had them stashed. I didn't re-add them because they were still there when I pulled changes

@DaWoblefet
Copy link
Member

As it is, this PR makes it so IVs are no longer auto-set in generations without hyper training on import, when they should be. You should using node build after making changes and testing the behavior via testclient.html.

I would strongly recommend testing the following:

In Generations without hyper training (Gens 2-6)

  • Importing a team without specific IVs and Hidden Power should automatically attempt to set appropriate Hidden Power IVs
  • IVs already set in the export should be respected on import and not modified

In generations with hyper training (Gen 7+)

  • Importing a team without specific IVs and Hidden Power should not automatically attempt to set Hidden Power IVs

Make sure to test for both Hidden Power [type] and just Hidden Power. Check for various ways of importing, e.g. importing a single team, importing teams in bulk, and importing from pokepaste.

@singiamtel
Copy link
Contributor

Make sure to clear cache between tests too

@EvGym
Copy link
Contributor Author

EvGym commented Aug 29, 2023

I did do that, Something is wrong with my local test copy and I am doing a deep purge and re-installing node to see if it will fix it. What I had thought was just standard reduced functionality with a local test turned out to be more severe than anticipated.

GOOD NEWS! Purging everything with fire and re-installing everything worked! Your guide was very helpful to ensure I didn't forget to test anything. I didn't even know you could import a team with just a pokepaste.

Problems:

  • The teambuilder is aggressively agnostic about formats. So it will only take it into account when importing from backup. I tried to route information about the format to it but I was not able to make any progress. People more experienced than I would need to do this. The server is more my speed.

Every other corrective measure for IVs when using hidden power still works. For example, editing your move set still changes IVs as it should.

So what does this actually change?

  • Sets directly imported into teams, or teams directly imported can no longer have their ivs tampered with. In any generation. (Importing your backups is left unchanged). The validator still catches this issue. You have always been able to import illegal sets through this method anyway so this won't catch people off guard.

@EvGym
Copy link
Contributor Author

EvGym commented Aug 29, 2023

I removed how the teambuilder would reset the IVs of mon that had them match the IV set for their hidden power type due to the blind spot with importing teams individually. This should make it so you can spam the import button on a team member in your roster and not have their IVs reset

@EvGym
Copy link
Contributor Author

EvGym commented Aug 30, 2023

One slight fix, during retesting I realized I forgot to include my fix for pokepaste imports

PR is ready for review again. Thank you all!

@EvGym
Copy link
Contributor Author

EvGym commented Nov 2, 2023

I am still watching this by the way. Hopefully, this will be able to be merged soon as this bug is extremely frustrating.

@EvGym
Copy link
Contributor Author

EvGym commented Dec 10, 2023

bumping this. I would still love to see this bug finally get fixed

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.

4 participants