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

Update to 1.7.2 GrassCutter w/ fixes and new features. #2

Merged
merged 9 commits into from
Oct 19, 2023

Conversation

mostm
Copy link
Contributor

@mostm mostm commented Oct 18, 2023

Thanks for your work on this plugin!
I tried here to fix issues with my GOOD scanned from official servers.
Was dissapointed to see missing characters and weapon import (main reason why I noticed your project), and implemented that myself.
Weapon import should be more futureproof, and does not require internal resources like your original version of artifact/character import, although I haven't tested that thoroughly with all weapons.

Copy link
Owner

@Penelopeep Penelopeep left a comment

Choose a reason for hiding this comment

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

Seems good, do you know of any other errors?
I'm asking since don't maintain it especially since GrasscutterCommandGenerator have similiar function

@Penelopeep
Copy link
Owner

Also I really appreciate your work, it's rare to see someone asking for my plugins

@mostm
Copy link
Contributor Author

mostm commented Oct 19, 2023

do you know of any other errors?

No, at least not with my export.
After my fixes, everything imports correctly
(according to GOOD database submitted), even items that Inventory Kamera incorrectly scanned (like the artifacts with 0 substats). Snoo's code will complain with this, but in the end - everything should be in place.
Feel free to try with my export - genshinData_GOOD_2023_10_15_12_57.json

since GrasscutterCommandGenerator have similiar function

I noticed that today, after I made all of the patches - before even testing server implementation xD

@Penelopeep
Copy link
Owner

If you say that there aren't any errors that you found then I guess it's good to go

@Penelopeep Penelopeep merged commit e77d7ed into Penelopeep:main Oct 19, 2023
@@ -67,12 +67,19 @@ public final class Import implements CommandHandler {
CommandHandler.sendMessage(targetPlayer,"Artifacts import disabled");
}
}

if (GenshinImporter.getPluginConfig().Weapons) {
Datareader.weapons(targetPlayer, filename);
Copy link
Owner

Choose a reason for hiding this comment

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

Also you forgot to add weapons function to datareader

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, seems that I've missed that when making commits to Git :(

@Penelopeep
Copy link
Owner

I'll fix that but Datareader doesn't have weapons function which is mentioned here

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.

None yet

2 participants