jjtParadox / Barometer

An old experimental test library for 1.12 MinecraftForge mods.
GNU Lesser General Public License v3.0
7 stars 3 forks source link

Update default settings to make a 'redstone ready' world #7

Closed tdaffin closed 6 years ago

tdaffin commented 6 years ago

This is one way to update the server.properties file as mentioned in this PR: https://github.com/jjtParadox/Barometer/pull/4.

Here I'm taking the approach of adding the setting from the code, as I noticed that other settings were being set that way.

However, I noticed this comment in the code right before the code that adds the settings: //TODO Allow all this stuff to be configured from build.gradle (or some better place?)

This prompts me to mention that another way to add the server properties would be in the 'test' task in the build.gradle file. For example:

    file("$workingDir/server.properties").text = """
level-type=FLAT
generator-settings=3;minecraft:bedrock,3*minecraft:stone,52*minecraft:sandstone;2;
online-mode=false
level-name=integrateddynamics.test
"""

To support this, I made a change to the hard-coded settings so that they will not overwrite the user's options in this case. I think it would be convenient for the code to generate good settings by default, but not overwrite settings that may have already been placed there by build.gradle.

Perhaps I should remove this piece of code now?:

//TODO Make a nice empty world for tests
//    @Mod.EventHandler
//    fun init(event: FMLInitializationEvent) {
//        TestWorldType()
//    }

Also, should I remove the //TODO Allow all this stuff to be configured from build.gradle (or some better place?), as perhaps this PR addresses that?

FYI: The particular generator-settings string I'm using here is the one used for the 'Redstone Ready' preset that ships with the base game. I prefer this to just filling everything with air, but your mileage may vary ;)

tdaffin commented 6 years ago

In case it helps, I've discovered that the built in preset called 'The Void' uses this setting (in 1.12 anyway): 3;minecraft:air;127;decoration The addition of 'decoration' adds a 1 block thick stone platform for a chunk or so around the player, so it may be worth including.

jjtParadox commented 6 years ago

Looks good!

For the default world, I'd set the generator to 3;minecraft:air;127; so it's a pure void (as there shouldn't be players joining in a test environment).

Go ahead and remove those two TODOs, this works well.

One thing: Does the server automatically generate server.properties before or after the preinit event? This could accidentally break things if it's generated before the preinit (or if a previously generated server.properties has some bad defaults). If it's generated afterwards I don't think there will be too much of an issue, but it's something to consider.

(Also, could you put a space before the { of safeSet? Helps keep code style consistent :smile:)

tdaffin commented 6 years ago

I've made the suggested changes. server.properties is generated after the line that reads it in preInt: val serverSettings = PropertyManager(File("server.properties"))