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

Add New Plugin "Meshy" #625

Merged
merged 6 commits into from
Oct 12, 2024
Merged

Add New Plugin "Meshy" #625

merged 6 commits into from
Oct 12, 2024

Conversation

Shadowkitten47
Copy link
Contributor

@Shadowkitten47 Shadowkitten47 commented Sep 28, 2024

Added "Meshy" plugin.

Adds support for the poly mesh format for bedrock entities ( new and old )
for exact details look at the read me

@Shadowkitten47
Copy link
Contributor Author

currently overwrites codec.parse and codec.compile haven't made the switch to events might do that in the future

Copy link
Owner

@JannisX11 JannisX11 left a comment

Choose a reason for hiding this comment

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

The mesh import and export seems to work quite well. I just have some notes regarding how things like settings are set up.

(function() {
const pluginInfo = {"name":"Meshy","id":"meshy","version":"1.0.2","repository":"https://github.com/Shadowkitten47/Meshy"};

if (!settings["normalized_uvs"])
Copy link
Owner

Choose a reason for hiding this comment

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

Settings IDs are very generic right now. These should be namespaced (e.g. meshy_normalized_uvs) to avoid conflicts with other plugins. The same applies to names. When looking through the settings, users will have no idea what "Meta Data" refers to without context.

plugin: pluginInfo.id
})
}
if (!settings["Force Multi-Textures"])
Copy link
Owner

Choose a reason for hiding this comment

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

This uses name instead of ID.



if (!BarItems['quick_reload']) {
new Action('quick_reload', {
Copy link
Owner

Choose a reason for hiding this comment

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

It's recommended to move setting up the plugin into onload. That way actions will be registered as part of the plugin.

if (!settings["normalized_uvs"])
new Setting("normalized_uvs", {
name: "Normalize UVs",
description: "Normalize uvs on export",
Copy link
Owner

Choose a reason for hiding this comment

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

Suggestion to move these to the export category. They seem pretty lost in the General settings page right now.

bedrock_old.single_texture = !settings["single_texture"]?.value

},
onunload() {
Copy link
Owner

Choose a reason for hiding this comment

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

Settings and actions are not cleaned up when uninstalling. You can do this here via setting_xy.delete(); etc.

@@ -0,0 +1,83 @@
const pluginInfo = {
Copy link
Owner

Choose a reason for hiding this comment

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

Do these source files include a build script? Or how would third party contributions work?

@Shadowkitten47
Copy link
Contributor Author

@JannisX11 I fix everything you mentioned. I removed the src sense it's human readable and really wasn't necessary ( You said I could but I had forgotten ). For 3rd Party contributions I want them to go to my own repo sense that's where I have been working on it. In that is the src and bundler GitHub repository. I made a few changes as well that you recommended

@JannisX11
Copy link
Owner

Please make sure that plugin meta data in the plugin itself and in plugins.json match. The plugin is currently missing some fields such as tags, min_version and creation_date.

@Shadowkitten47
Copy link
Contributor Author

@JannisX11 Done!

@JannisX11 JannisX11 merged commit c072417 into JannisX11:master Oct 12, 2024
1 check failed
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.

2 participants