Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: ✨ Config JSON: Only log notice for no custom JSON once #138

Merged

Conversation

ithinkandicode
Copy link
Collaborator

When the config JSON code wants to log that it's tried to find a custom JSON file but couldn't find one, it will not only log a notice to say this once.

Also adds a new array variable to mod_loader called logged_messages. Config JSON's missing JSON messages are pushed to this array, which lets us check to see if the exact message was already logged.

Closes #133:

@ithinkandicode ithinkandicode added the validation Feature to make things safe and predictable label Feb 26, 2023
@ithinkandicode ithinkandicode linked an issue Feb 27, 2023 that may be closed by this pull request
@ithinkandicode ithinkandicode linked an issue Feb 27, 2023 that may be closed by this pull request
@KANAjetzt KANAjetzt requested a review from Qubus0 February 27, 2023 11:38
@Qubus0
Copy link
Collaborator

Qubus0 commented Feb 27, 2023

for performance, it could be worth to store all the messages in a dictionary with the keys being their md5 version.
A it doesn't have to look for the entire message every time
B dictionary lookup is way faster
C we can still get all the messages by just using .values()

    print("Config JSON Notice: something something".md5_text())     # b1f526f21bb565e91f11096f3cc1b499
    print("Config JSON Notice: something something".md5_text())     # b1f526f21bb565e91f11096f3cc1b499 - same
    print("abc Config JSON Notice: something something".md5_text()) # 34b0bb58ba94c388f7dd361ec0836172 - not same

@ithinkandicode
Copy link
Collaborator Author

@Qubus0 That's a good idea but I think it might be better to suggest that on issue #140?

When 140 is implemented, the code here will change anyway

Copy link
Collaborator

@Qubus0 Qubus0 left a comment

Choose a reason for hiding this comment

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

Then let's fast forward to #140

@ithinkandicode ithinkandicode merged commit 01983b7 into GodotModding:development Feb 28, 2023
@ithinkandicode ithinkandicode deleted the config-json-limit-logs branch February 28, 2023 22:47
@ithinkandicode ithinkandicode changed the title Config JSON: Only log notice for no custom JSON once feat: ✨ Config JSON: Only log notice for no custom JSON once Jun 18, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
validation Feature to make things safe and predictable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config JSON: Limit logging when there's no custom JSON file
2 participants