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

Greenhouse not always working. #12

Closed
Langly- opened this issue May 1, 2015 · 34 comments
Closed

Greenhouse not always working. #12

Langly- opened this issue May 1, 2015 · 34 comments

Comments

@Langly-
Copy link

Langly- commented May 1, 2015

Greenhouse seems to only be ignoring biome growth rates in biomes where something can grow. You can get faster rates for biomes a crop grows very slowly in, but it won't work at all in a biome something can not grow in. That is not ignoring biome, and something is odd.

Secondly, the config spreadsheet on txapu has been showing Dark Oak should now grow in mushroom biomes, that does not work.

@ttk2
Copy link

ttk2 commented May 1, 2015

this is the right place, thanks.

On Fri, May 1, 2015 at 1:38 PM Langly- notifications@github.com wrote:

Greenhouse seems to only be ignoring biome growth rates in biomes where
something can grow. You can get faster rates for biomes a crop grows very
slowly in, but it won't work at all in a biome something can not grow in.
That is not ignoring biome, and something is odd.

Secondly, the config spreadsheet on txapu has been showing Dark Oak should
now grow in mushroom biomes, that does not work.


Reply to this email directly or view it on GitHub
#12.

@WildWeazel
Copy link

It does ignore biomes, because glowstone and biome modifiers are calculated independently. It simply replaces sunlight with a 75% modifier. The alternative would be to override the biome rate with the glowstone rate. It's a poorly named variable that isn't being used.

@Langly-
Copy link
Author

Langly- commented May 9, 2015

The biome rate was being overridden in some cases, potato grow faster in
plains with glowstone than they do in sunlight. Overriding biome rate would
be the best and make the most sense, since that would be realistic as far
as growing things go. And thanks.

On Fri, May 1, 2015 at 1:27 PM, Travis Christian notifications@github.com
wrote:

It does ignore biomes, because glowstone and biome modifiers are
calculated independently. It simply replaces sunlight with a 75% modifier.
The alternative would be to override the biome rate with the glowstone
rate. It's a poorly named variable that isn't being used.


Reply to this email directly or view it on GitHub
#12 (comment)
.

@ttk2
Copy link

ttk2 commented May 9, 2015

whats needed to fix this?

On Sat, May 9, 2015 at 12:15 PM Langly- notifications@github.com wrote:

The biome rate was being overridden in some cases, potato grow faster in
plains with glowstone than they do in sunlight. Overriding biome rate would
be the best and make the most sense, since that would be realistic as far
as growing things go. And thanks.

On Fri, May 1, 2015 at 1:27 PM, Travis Christian <notifications@github.com

wrote:

It does ignore biomes, because glowstone and biome modifiers are
calculated independently. It simply replaces sunlight with a 75%
modifier.
The alternative would be to override the biome rate with the glowstone
rate. It's a poorly named variable that isn't being used.


Reply to this email directly or view it on GitHub
<
#12 (comment)

.


Reply to this email directly or view it on GitHub
#12 (comment)
.

@WildWeazel
Copy link

If you can, get some screenshots with debug mode and the growth rate message for both cases.

@plebes
Copy link

plebes commented May 30, 2015

I would like to fix this problem, and I think I know what it is. I just want to make sure that the greenhouse feature is where crops adjacent to glowstone faces have the possibility of growing if configured to do so in the config.

Also, regarding the getRate() function in GrowthConfig, if a crop does not require sunlight to grow, should it still incur a penalty for not being at max sunlight level? (This is the case currently).

Once I know, I will commit.

Edit: Langly is correct in his observation. Potatos take 6 hours in plain under full sunlight, and 4 hours in the same spot with just glowstone.

@ttk2
Copy link

ttk2 commented May 30, 2015

yes that is how the greenhouse function should work as far as I know.

If the crop does not require sun, it should not get a penalty, although
this might lead us to making some crops that don't currently require run
require it if they slipped through the cracks due to this.

I hope that answers your quesitons, just ask if you need any help at all.

On Sat, May 30, 2015 at 2:43 AM plebes notifications@github.com wrote:

I would like to fix this problem, and I think I know what it is. I just
want to make sure that the greenhouse feature is where crops adjacent
to glowstone faces have the possibility of growing if configured to do so
in the config.

Also, regarding the getRate() function in GrowthConfig, if a crop does not
require sunlight to grow, should it still incur a penalty for not being at
max sunlight level? (This is the case currently).

Once I know, I will commit.


Reply to this email directly or view it on GitHub
#12 (comment)
.

@WildWeazel
Copy link

I suppose the way it should work is if glowstone is found, take the greater of 75% or the calculated sunlight rate. Not sure what the intention was for non-sunlight, but they should probably not grow any faster with more/full light.

@benjajaja
Copy link

@Langly- I will update http://txapu.com/rb.html to show growth times/percentage with glowstone too, mostly for usability, I'd like to know growth for greenhouse too ;)

Otherwise, I think the behaviour is correct - if something cannot grow at all in a biome, glowstone shouldn't work either, or you could just grow A LOT of things in most biomes. You could suddenly grow most crops and trees in an ocean biome!

@ProgrammerDan
Copy link

It'd be kind of neat to allow crop growth out of biome with glowstone, but that would be a pretty big feature break.

Imagine 1%-5% of "allowed" biome growth rate, but only in a greenhouse. It'd take a week to grow anything :)

@Langly-
Copy link
Author

Langly- commented Jun 12, 2015

It would make sense for glowstone to work in other biomes though. If you
built a a heated lit cavern in Antarctica you could still get stuff to
grow. Growing things with glowstone gets costly, and would still limit what
could be done in other biomes. It would also let ocean cities be a bit more
feasible, yet still costly to grow crops in.

On Mon, Jun 8, 2015 at 8:38 AM, Benjamin Grosse notifications@github.com
wrote:

@Langly- https://github.com/Langly- I will update
http://txapu.com/rb.html to show growth times/percentage with glowstone
too, mostly for usability, I'd like to know growth for greenhouse too ;)

Otherwise, I think the behaviour is correct - if something cannot grow at
all in a biome, glowstone shouldn't work either, or you could just grow A
LOT of things in most biomes. You could suddenly grow most crops and trees
in an ocean biome!


Reply to this email directly or view it on GitHub
#12 (comment)
.

@ProgrammerDan
Copy link

Maybe we should open a Discourse on this?

@benjajaja
Copy link

Go ahead, I will argue for LOWERING greenhouse efficiency >:)

@ProgrammerDan
Copy link

I'd agree with you! :P

Edit: and then turn around w/ Langly's argument hehe.

@Langly-
Copy link
Author

Langly- commented Jun 12, 2015

Lower the efficiency, but allow it to work anywhere. Meaning limited but
possible crops. That would work as a nice even balance.

On Fri, Jun 12, 2015 at 9:03 AM, Daniel Boston notifications@github.com
wrote:

I'd agree with you! :P


Reply to this email directly or view it on GitHub
#12 (comment)
.

@benjajaja
Copy link

@Langly- Then at once, location/biome would be pretty meaningless, and a massive amount of people that are using existing greenhouse ratios would be very upset?

@ProgrammerDan
Copy link

I'm thinking from a balance perspective, volume alone and general
availability of glowstone (1DC was publicly traded by a single individual
on Wednesday in the space of two hours with promise of more easily
available) demands something like 25-50x less efficient in greenhouses,
regardless of biome, but I'd expect like 25x less efficient in allowed
biome, and 50x less efficient in "non-standard" biome.

Basically, you can grow crops "anywhere" in a greenhouse, but you'll need a
farm 50x times as large as just grabbing a plot within biome, and you'll
need enough glowstone to meet the adjacency requirement for that whole 50x
large field.

Might be missing something, but sounds like a good tradeoff.

On Fri, Jun 12, 2015 at 12:49 PM, Langly- notifications@github.com wrote:

Lower the efficiency, but allow it to work anywhere. Meaning limited but
possible crops. That would work as a nice even balance.

On Fri, Jun 12, 2015 at 9:03 AM, Daniel Boston notifications@github.com
wrote:

I'd agree with you! :P


Reply to this email directly or view it on GitHub
<
#12 (comment)

.


Reply to this email directly or view it on GitHub
#12 (comment)
.

@ProgrammerDan
Copy link

@gipsy-king Let them eat cake! (still, valid objection)

@benjajaja
Copy link

People will build anything if they can get around dealing (especially economically) with other people. No matter how large they must make their farm or what it costs, they will build it if it means they don't have to travel or trade with others. Also, we're getting compactors soon unless I'm mistaken, so that's one more point against this.

Greenhouse defeats the purpose of RB at the current rate. But I wouldn't really argue for lowering the rate, because that would just suck for all the players who have already built large greenhouse farms. Maybe reduce it from 75% to 60% or something, with the introduction of compactors being the excuse.

The enabling of greenhouse in ANY biome, IF done, should at most be at like 1%? So that you can grow for decorative purposes only, nothing else.

@ProgrammerDan
Copy link

The enabling of greenhouse in ANY biome, IF done, should at most be at like 1%? So that you can grow for decorative purposes only, nothing else.

I think we're well into the territory of wishful thinking, because you've hit on the core issue -- greenhouse defeating the purpose of having value within specific biomes if we open that floodgate at all. Most suggestions on rebalance completely miss the total focus and absolutely maniacal devotion our playerbase has to subvert the goals and go lone wolf or game the system.

Even as I wrote about 50x reduction in output (like .5% of biome-growth) I was envisioning construction of a MASSIVE 512x512x60 multi-level farm, employing armies of bots to harvest each level, built into the ocean. At that size it'd more than make up for the biome growth reduction, although at great cost.

@WildWeazel
Copy link

In the original Civtest where this was introduced we did allow glowstone to override biome settings, and it was evident that if it was powerful enough to be worth using at industrial scale, it would make biomes irrelevant and defeat the purpose of the plugin.

@ProgrammerDan
Copy link

@plebes We've hijacked your thread with nonsense. Do you need any assist on moving this patch forward?

@plebes
Copy link

plebes commented Jun 13, 2015

Hi Dan,

I re-wrote the getRate function with a fix to the logic and rate calculation. However, I didn't issue a pull request because I would like to test the changes on a local build of the realistic biomes plugin before the pull request.

I set up a bukkit server, and have written and tested my own plugins (a change to the anvil). However, I've not been able to do so with realistic biomes.

If you, or someone else, can help me get realistic biomes running with the sql server locally, then I can continue the bug fixes.

My goal was to contribute to the civcraft server by fixing the bugs, as it is fun, and useful. I picked realistic biomes because no one had been working on the bugs, and I found some quite easily.

If I get a local build of the plugin working, then I will fix the bugs and add whatever else is wanted.

Thanks.

@benjajaja
Copy link

@plebes you just need both a database and user called "tekkit", just give the user all privileges.

Regarding getRate, there is no bug as far as I understand. It is too complex, yes. A rewrite is tricky, because the server really depends on the exact implementation. I wouldn't rewrite this one without writing a test 😮

@ttk2
Copy link

ttk2 commented Jun 13, 2015

RB in its current state is not doing as much as it perhaps should to
achieve the goals of biome specific production, especially since biomes are
small enough to allow easy travel between them and the setup of multiple
farms, not totally sure if these concerns are super relevent, we don't
exactly want to make things worse but at the same time they can't get too
much worse and our major concern should be if this interferes with our
ability to fix things.

Plebs, thanks for the coding work! Sorry you accidentally chose a
contentious issue to start with, it happens, at the very least having the
feature working again will be appreciated even if it remains off. Just let
me know if you need anything.

On Sat, Jun 13, 2015 at 4:07 AM Benjamin Grosse notifications@github.com
wrote:

@plebes https://github.com/plebes you just need both a database and
user called "tekkit", just give the user all privileges.

Regarding getRate, there is no bug as far as I understand. It is too
complex, yes. A rewrite is tricky, because the server really depends on the
exact implementation. I wouldn't rewrite this one without writing a test [image:
😮]


Reply to this email directly or view it on GitHub
#12 (comment)
.

@plebes
Copy link

plebes commented Jun 13, 2015

@gipsy-king Thank you for the information. I thought that I had to run a script to set up all of the tables first. If it is created automatically, then that is great.

Also, there does seem to be a logic error in the getRate function. All crops, even if they do not require sunlight, have a penalty if they are not in full sunlight. Look at the conditional statement on line 219. It would seem that it should be nested inside the conditional for crops that need sunlight on 215.

Also, If you go into the plains biome (for instance), you will see that crops (potato) grow faster with the glowstone greenhouse feature than the same crop in full sunlight, which is what I was asking about previously.

@ttk2 I will try to get the database working with the realistic biomes plugin and then help out more.

In addition, when sharding is incorporated, couldn't you do some new feature such as having biomes in the shards having independent growing weights that can be cycled? If you have 7 shards, then you can cycle a "shard weight" amongst those shards. So sometimes one shard has the best growth, and the next another. You can cycle it each day or one a week. That might encourage trade as well since one groups shard may have very poor growth for the week, while their allies has the best growth. Basically, it allows you to simulate sunlight moving over a planet and get around the minecraft universal time for all chunks.

@benjajaja
Copy link

@plebes I didn't write RB but I did make http://txapu.com/rb.html so I will try to give you an explanation on those lines of code:

The only crops that don't require sunlight but do have a notFullSunlightMultiplier are nether warts, which is the only crop that has that set as boost, not penalty! notFullSunlightMultiplier default value is 1, so if not set it does nothing.

This is just one example of why you are obvisouly correct about it all being an ungodly mess, both for coders, config-file editors, and end users that try to make sense of it.

While working on it today, I was contemplating throwing it away and rewriting everything, but that would of course require a huge deal of testing. What do you think about waiting for me to implement persistence for "fruitful" crops (crops that spawn blocks: melon, pumpkin, cactus, sugarcane, mushroom and sapling), and then try a complete rewrite? I know it may sound like a waste of my time, but it would help a lot if we knew the real requirements and pitfalls of these fruitful crops.

@plebes
Copy link

plebes commented Jun 13, 2015

@gipsy-king It would be fun from a coding perspective to do a full rewrite, but I don't think it would be best for Civcraft itself. In my opinion, if we contribute, it would be best to fix the bugs currently known, which are very few, than to write an entirely new version of this plugin. However, if you have a design that has improved performance in terms of structure or algorithms used, then a rewrite would be good. I only speak for my own opinion in that I want to contribute by fixing the bugs, and improving the code in terms of safety at the same time. To be honest, my time is limited, so this is the best way to contribute for me.

If you do begin a new version of RB, then please do let me know about your repo.

Thanks

@benjajaja
Copy link

@plebes no, it was just an idea.

As I said I'm working on persisting these new crops right now, I haven't touched GrowthConfig.java and I don't think I will have to, because I don't think it is buggy per se - it's just counterintuitive and extremely difficult to make out. I think if you changed getRate, you would have to adapt the config too, so that it all ends up understandable without going through the code.

@plebes
Copy link

plebes commented Jun 13, 2015

@gipsy-king Currently crops like potato can grow faster in the plains with glowstone than with full sunlight, which is what I think they said to change up above. If you can change that, then the get rate function is confusingly coded, but results will be fine. I think I will stay away from get rate right now since you seem to know it better than me.

@benjajaja
Copy link

potato grows faster in plains with glowstone - sounds correct, without glowstone it's 0.5 efficiency (it's 1.0 in mountains), with glowstone you should get 0.75 efficiency which would be 6h vs 4h30min in plains I think from what I can gather at http://txapu.com/rb.html

@ttk2
Copy link

ttk2 commented Jun 13, 2015

Feel free to refactor as much as you need to and try to clean things up.
Unless you go into rewrites with very strict rules about what you are out
to fix it usually does not help as much as people think it would.

On Sat, Jun 13, 2015, 3:03 PM plebes notifications@github.com wrote:

@gipsy-king https://github.com/gipsy-king Currently crops like potato
can grow faster in the plains with a glowstone than with full sunlight,
which is what I think they said to change up above. If you can change than,
then the get rate function is confusingly coded, but results will be fine.
I think I will stay away from get rate right now since you seem to know it
better than me.


Reply to this email directly or view it on GitHub
#12 (comment)
.

@benjajaja
Copy link

@Langly- @plebes can we close this?

@benjajaja
Copy link

Closing since greenhouse appears to be working as described in the wiki.
@Langly- if the dark oak issue still isn't solved, reopen this issue.

psygate pushed a commit to psygate/RealisticBiomes that referenced this issue Oct 26, 2020
GUI showing crop growth times for current biome
psygate pushed a commit to psygate/RealisticBiomes that referenced this issue Jun 23, 2022
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

No branches or pull requests

6 participants