Add a bunch of new modifiers and compatibilities with mod Ars Elemental

wendall911 / TCIntegrations

A Minecraft 1.18+ mod designed to provide modpack integrations with other mods for Tinkers' Construct

MIT License

3 stars 5 forks source link

Add a bunch of new modifiers and compatibilities with mod Ars Elemental #65

Closed Hophenby closed 4 months ago

Hophenby commented 4 months ago

Hi there, I'm a big fan of tinkers construct and your project. I've known about Chinese community shows great interest of adding RF modifiers so i worked a lot these days to add some of them to the game.

i took much time getting around with tc's modifier system, and it finally seems to be working well though

i'm also adding the compatibilities with ars elemental (a popular ars nouveau addon) to the mod.

(with 4 elements) this shall be an upgrade (but consumes an ability slot) of archmage modifier. zh_cn.json also upgraded

wendall911 commented 4 months ago

Hey, appreciate the PR. I will need to spend some time and review this. There are many issues with this PR, including changes to the build file that are essential for my uses that absolutely need to not be removed. I can probably extract the working code from this and test, but it will take some time. This isn't usable as-is, but I will comment once I can extract working code and test a bit.

In the future, please do not delete things out of the build file, as it is essential for publishing and testing.

One other issue is that the updated translation file will need to be verified by another native speaker. I have a couple folks who have done translations for our mods if you don't know anyone. https://github.com/wendall911/TCIntegrations/pull/65/commits/f4dfa4c145fdab0f4207c63f432ad24b787fe9e6

Hophenby commented 4 months ago

thanks a lot. the build file changes as i failed to load maven repo from githubpakages and to prevent accidentally publishing or misclicking. sorry for that!

wendall911 commented 4 months ago

Ok, after looking this over, I can't accept it as-is, but I can extract the code and give credit to you for the contribution. There are too many things that need to be changed to ask for a simple edit and resubmission. I might be able to use some of the commits, but will need to edit most things.

wendall911 commented 4 months ago

There are too many issues with this PR to accept it. Needs a lot of cleanup. I see there is a sound file that is registered, but no licensing information about the file.

Given I have to maintain the features, it would need a lot of changes and match coding styles and naming to be useful for me to maintain.

For me to be able to accept Pull Requests, they need to be smaller and at minimum match the existing coding style. Making build file changes, adding unlicensed sound files and language translations is not something I can work around until it gets resolved.

Maybe if I have some time later I will clean this up and use some of it. But for now I need to close this. I do appreciate the effort though. I"m sure it works for you. Just not something I can justify working on.

Hophenby commented 4 months ago

that's no problem. thanks for suggestions :)