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

Updating labeling on misleading Configuration params #65

Closed ProgrammerDan closed 8 years ago

ProgrammerDan commented 8 years ago

These config parameters are badly misrepresented.

The call used: https://hub.spigotmc.org/javadocs/spigot/org/bukkit/scheduler/BukkitScheduler.html#runTaskTimer%28org.bukkit.plugin.Plugin,%20java.lang.Runnable,%20long,%20long%29

Batch Period is the delay for first execution. BatchMaxTime is the period -- according to the current implementation.

The run helper used doesn't limit execution time of the task.

Above label changes reflect the true functioning of these components instead of the misleading comments.

CivcraftBot commented 8 years ago

Can one of the admins verify this patch? Type 'ok to test' to test.

Maxopoly commented 8 years ago

I feel like we should just rename the option and adjust the config parser instead of having a comment saying an option is not what its name literally says. That's just confusing

ProgrammerDan commented 8 years ago

Yeah totally agree. This was largely to raise visibility. Shoulda just opened an issue but I'm tired.

ttk2 commented 8 years ago

so merge?

ProgrammerDan commented 8 years ago

Nah, lemme do the actual fix. Consider this a placeholder, do not merge. I'll pull a Rourke and make a real patch later today :)

WildWeazel commented 8 years ago

Did this ever get done? There were several misleading parameter names in there.

Maxopoly commented 8 years ago

Considering that the last commit is older than this isse, probably not

ProgrammerDan commented 8 years ago

I'm going to just merge this, and open an issue pointing to the config options for later fixing.