Skip to content

Fix GenericSetting and Vector3dSetting not calling onChanged#5780

Merged
Wide-Cat merged 1 commit into
MeteorDevelopment:masterfrom
mankool0:fix-generic-setting-onchanged
Oct 21, 2025
Merged

Fix GenericSetting and Vector3dSetting not calling onChanged#5780
Wide-Cat merged 1 commit into
MeteorDevelopment:masterfrom
mankool0:fix-generic-setting-onchanged

Conversation

@mankool0

Copy link
Copy Markdown
Contributor

Type of change

  • Bug fix
  • New feature

Description

  • Added screen.onClosed() callback in genericW() to call setting.onChanged()
  • Fixed addVectorComponent() to call setting.onChanged()
  • Fixed ESPBlockDataScreen.onChanged() to always call setting.onChanged(), not just on first edit

How Has This Been Tested?

Logging to verify onChanged() callbacks fire correctly.

Checklist:

  • My code follows the style guidelines of this project.
  • I have added comments to my code in more complex areas.
  • I have tested the code in both development and production environments.

@crosby-moe crosby-moe mentioned this pull request Oct 19, 2025
5 tasks

@crosby-moe crosby-moe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the changes for Vector3dSetting are correct, however the changes to GenericSetting would have resulted in the onChanged consumer being called without any changes actually made to the setting

fixing this would have required some refactoring, so i have taken the liberty of doing it all myself in #5787

@Wide-Cat

Copy link
Copy Markdown
Collaborator

closed in favour of #5787

@Wide-Cat Wide-Cat closed this Oct 21, 2025
@crosby-moe crosby-moe reopened this Oct 21, 2025
@crosby-moe

Copy link
Copy Markdown
Collaborator

Vector3dSetting changes are still valid

@mankool0 mankool0 force-pushed the fix-generic-setting-onchanged branch from cc6cca6 to 830656a Compare October 21, 2025 04:21
@mankool0

Copy link
Copy Markdown
Contributor Author

I've removed my GenericSetting changes, so hopefully it's good now.

@Wide-Cat Wide-Cat merged commit 31717aa into MeteorDevelopment:master Oct 21, 2025
1 check passed
MistressOfDNS pushed a commit to MistressOfDNS/meteor-client-fork that referenced this pull request May 7, 2026
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.

3 participants