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

fix(skymp5-server): fix missing 'numChanges' in equipmentDump JSON #2235

Merged
merged 2 commits into from
Nov 29, 2024

Conversation

Pospelove
Copy link
Contributor

@Pospelove Pospelove commented Nov 28, 2024

Important

Add logging for missing 'numChanges' key in equipmentDump JSON in MpChangeForm::JsonToChangeForm.

  • Behavior:
    • In MpChangeForm::JsonToChangeForm, if equipmentDump JSON lacks numChanges, it is set to 0 and logged using spdlog.
  • Logging:
    • Added spdlog include to MpChangeForms.cpp for logging purposes.

This description was created by Ellipsis for 78290eb. It will automatically update as commits are pushed.

@ellipsis-dev ellipsis-dev bot changed the title . fix: log missing 'numChanges' in equipmentDump JSON Nov 28, 2024
@Pospelove Pospelove changed the title fix: log missing 'numChanges' in equipmentDump JSON fix(skymp5-server): fix missing 'numChanges' in equipmentDump JSON Nov 28, 2024
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 78290eb in 32 seconds

More details
  • Looked at 27 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_oCrXFIS3OGBmvpMv


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -192,6 +193,15 @@ MpChangeForm MpChangeForm::JsonToChangeForm(simdjson::dom::element& element)
res.equipmentDump = simdjson::minify(jTmp);
if (res.equipmentDump == "null") {
res.equipmentDump.clear();
} else {
auto equipment = nlohmann::json::parse(res.equipmentDump);
Copy link

Choose a reason for hiding this comment

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

Consider adding exception handling around nlohmann::json::parse to handle potential parsing errors gracefully.

@Pospelove Pospelove merged commit 5b3dc82 into skyrim-multiplayer:main Nov 29, 2024
10 checks passed
@Pospelove Pospelove deleted the fixeq branch November 29, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant