Civcraft / RealisticBiomes

Do not open issues here; open them on the maintained fork @ DevotedMC
https://github.com/DevotedMC/RealisticBiomes
7 stars 14 forks source link

Glowstone multiplier change #40

Closed Maxopoly closed 9 years ago

Maxopoly commented 9 years ago

Changing the way glowstone works, so it multiplies the biome growth rate instead of the base growth rate to prevent players from stacking farms in a 0% growthrate biome thanks to glowstone.

https://forum.civcraft.co/t/making-glowstone-multiply-the-growth-rate-of-the-biome-instead-of-the-base-rate/697

benjajaja commented 9 years ago

What about trees? They still have greenhouse_ignore_biome: true.

I would also set greenhouse_rate to 0.25 but that should be a separate PR since this is more of a bug fix.

ttk2 commented 9 years ago

bumping this.

benjajaja commented 9 years ago

@Maxopoly What about trees, should they be growable in any biome? That could be reasonable maybe for decorative purposes, but then I think the green-house rate for them should be lowered. What do you think?

Maxopoly commented 9 years ago

I think the current config for trees is very balanced, I think we should give it the same treatment as the other crops and just remove the possibility of using glowstone to grow it anywhere. I'll update the pulll request towards that.

ttk2 commented 9 years ago

@rourke750 thoughts? I want to merge this

WildWeazel commented 9 years ago

I'm still not clear on whether something has changed to give glowstone any effect in a bad biome. The way it was supposed to work, this would have the opposite effect of what I think you guys want to do.

Maxopoly commented 9 years ago

Why? The current config is:

# if this flag is true, then the greenhouse growth will also ignore biome growth rates
   greenhouse_ignore_biome: true

Nothing has changed to give glowstone any effect in a bad biome as far as I understand it, it has always been like this.

WildWeazel commented 9 years ago

Because the description of that setting is very misleading. It means the glowstone rate and biome rates are factored independently, so adding glowstone has no effect on the biome. If it's false, then glowstone replaces the biome rate and will provide greenhouse_rate everywhere. So making this false will allow glowstone to grow all crops in all biomes. b96abb9ad14b6bed52e2e54ae6005cd9614420e9

Maxopoly commented 9 years ago

Hmm okay, so thats the wrong way. That still doesn't explain why people told me they used glowstone to grow stuff whereever they wanted. I just planted some crops on civtest to just test it on my own and I'll talk to a few people again if I catch them online in mumble.

benjajaja commented 9 years ago

@WildWeazel @Maxopoly the growth-rate calculation is still very confusing to me.

At this line: https://github.com/Civcraft/RealisticBiomes/blob/master/src/com/untamedears/realisticbiomes/GrowthConfig.java#L320

if greenhouse_ignore_biome is true then glowstone overrides biome-rate, if improving. You can grow a plant ANYWHERE with glowstone. At a rate of 0.75 for most plants.

if greenhouse_ignore_biome is false then glowstone cannot override zero-growth biomes (1), only bad sunlight conditions. You can grow things better under bad light conditions OR bad but not zero-growth biomes.

(1): It sets rate to zero earlier so nothing more can multiply it. This is very confusing, the function should exit right awayif zero!

This is the way I read it works, please correct me if I'm wrong.

ttk2 commented 9 years ago

we should probably just clean up and simplify how it works, if we need to have multi-day debates on how the code works its obviously horrible

Maxopoly commented 9 years ago

The RB config doesn't specify a growth rate for the biomes where something can't grow, so this part simply returns 0.0, if the local growthrate is 0/not in the config (line 279)

if (biomeMultiplier != null) {
            environmentMultiplier *= biomeMultiplier.floatValue();
        } else {
            return 0.0; // if the biome cannot be found, assume zero
        }

So it seems like you shouldn't be able to grow something even with glowstone,if the rate is 0. I'm testing this on my own again and if it doesn't work, we either have to assume whoever told me they were stacking farms was reading the config wrong or lying, idk.

benjajaja commented 9 years ago

@Maxopoly here, biomeMultiplier appears to be a biome object which is null only if the given biome type is not configured in RB's list, not if it is not configured for the plant. I'm not 100% sure on this one, but since you can grow plants in biomes they aren't configured for, it has to be the way I'm explaining it.

ttk2 commented 9 years ago

bump, whats going on here?

Maxopoly commented 9 years ago

Gipsy set the bounds for a rewrite of the method in question, which would clean up the code and also solve the problem here: https://github.com/Civcraft/RealisticBiomes/issues/42