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

Better Intergration #40

Closed
GenDeathrow opened this issue Dec 11, 2016 · 14 comments
Closed

Better Intergration #40

GenDeathrow opened this issue Dec 11, 2016 · 14 comments
Assignees
Milestone

Comments

@GenDeathrow
Copy link
Collaborator

GenDeathrow commented Dec 11, 2016

Found a couple of things, when I was trying to add support to your mod, One of them being Parents get registered as soon as the Chicken is registered, only down side with this is it makes it hard to register a chickens parent out of order, or if say in configs someone wanted to change a chicken's parent to a different parent, they would have to stay in the loading order. Basically if chicken A and Chicken B are about to be registered but chicken C was added before A and B, Chicken C couldn't select A or B cause they aren't in the registry yet. I am thinking that if you register the chickens but register their parents after all chickens have been added. perhaps, register chickens where they are at now, and register parents on POST loadup.

https://github.com/setycz/ChickensMod/blob/1.10.2/src/main/java/com/setycz/chickens/ChickensMod.java#L146-L152

Also Version numbers are incorrect for the @mod annotation. Last time I checked I think you added - "MCVersion - ModVersion". More chickens is dependent on Chickens mod, but I cant make sure its dependent on a specific version because of the dash" The Version number should be just the ModVersion only with no letters or special characters. basically "1.0.0" or "1.0".

@setycz
Copy link
Owner

setycz commented Dec 11, 2016

Hi. Thank you for you suggestions, I really appreciate that. Let's continue with the first think in this thread and that's the chicken registration. I have created #41 for the mod version issue.

@setycz
Copy link
Owner

setycz commented Dec 11, 2016

BTW, what do you think about MC 1.9.4, do you think that it make any sense to keep it updated? I would like to know your opinion.

@GenDeathrow
Copy link
Collaborator Author

To be honest, Most ppl just updated from 1.9.4 to 1.10.2. I don't see a reason to keep the 1.9.4 updated. And more than likely the 1.10.2 version will still work on 1.9.4 unless you happen to call a new method. I will be busy today, but we can discuss more on the chicken registry later.

@setycz
Copy link
Owner

setycz commented Dec 13, 2016

The chickens registry, it was designed in a way that it won't allow to have 'children' without both 'parents'. So as you said, parents need to be in the registry first. I haven't realised that it may be to strict and that it could cause hard times for addon developers. I think I can change it to be flexible and e.g. do a validation in the post load event and possibly removing invalid combinations (chicken which any of its parents are not in the registry). What do you think?

@GenDeathrow
Copy link
Collaborator Author

I'm guessing you will store the parents String ID during configuration load than in post actually find the parents in the chicken registry. or something along those lines. Either way that will make sure every chicken is loaded and it should find the parent. if it exist.

Side note I am realizing as I go back though your code.. I could have done some things differently.. mainly I did some things I didn't need too. So I am going to rewrite parts of my code before posting it up.

I will try to keep up to date so More chickens wont be broken for the new version.

@GenDeathrow
Copy link
Collaborator Author

@setycz I may have over thought this, I'm thinking I can handle the parents on my side. I started to look at the code again, and realized I can just handle mine on my side believe. I think I can simply call a new method to go though and set all the parent's after the registering. I realized the fisrt time I wrote this I sort of rushed its. Missed a few things I could have done better. I will get back with ya.

@setycz
Copy link
Owner

setycz commented Dec 17, 2016

Ok sure. Let me know what you'll find out and based on that I can increase the priority if this issues slows your development down.

@setycz setycz self-assigned this Dec 17, 2016
@GenDeathrow
Copy link
Collaborator Author

https://github.com/GenDeathrow/MoreChickens I think were good on the issue with parents now. I had to change some things over, but it works now. Im still cleaning up the code, just because I did a lot of dumb things the first time. and I will be condensing the code down more. Some things are redundant

@setycz setycz added this to the 4.2 milestone Dec 20, 2016
@GenDeathrow
Copy link
Collaborator Author

Hey I noticed you deleted a method getAllItems() im getting a crash report with the newest update cause I had been calling that method. Not a big deal but could you @depreciate any method you wont be using and leave them in for an update or two.Just So I have time to fix it?

@GenDeathrow
Copy link
Collaborator Author

@GenDeathrow
Copy link
Collaborator Author

I do like how you separated the enabled/disabled chickens though.

@setycz
Copy link
Owner

setycz commented Dec 21, 2016

@GenDeathrow I have created a hotfix for the missing getAllItems(), it's great that your mod is now on GitHub so I can check what methods do you use.

@GenDeathrow
Copy link
Collaborator Author

I appreciate.. I did just release a fix for the new stuff anyways. probably not available yet. Hatchery also got an update to display your dna stats in the eggs. and drops inside the nesting pens with jei.

And I'm working around your mod for the most part, if you remove something that's ok in my book. I have your mod inside my dev environment. So if you make changes hopefully I will see them and can change as needed.

@setycz
Copy link
Owner

setycz commented Jan 1, 2017

Integration problem is handled in More Chickens mod, further improvements can be done later in a new issue.

@setycz setycz closed this as completed Jan 1, 2017
@setycz setycz removed the question label Jan 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants