Skip to content
This repository has been archived by the owner on Apr 18, 2020. It is now read-only.

Fabricization #4

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fabricization #4

wants to merge 5 commits into from

Conversation

i509VCB
Copy link

@i509VCB i509VCB commented Apr 17, 2020

This PR refactors much of LibGameRule to meet the API standards set for fabric's api. Namely things have been changed where GameRule types are created in the RuleFactory and are registered in the GameRuleRegistry.

This also adds a new TextRule which stores Text.
Other new tweaks such as now using brigadier's literals for enum rules so vanilla clients need not worry about missing argument types.

Of course this isn't complete yet, this is here to measure the progress towards a library of high enough quality to PR into fabric's API.

One issue is FabricMC/fabric-loom#193 where we can't use an invoker for creation of regular RuleTypes, but the workaround for this could stay.

@i509VCB i509VCB added the enhancement New feature or request label Apr 17, 2020
@i509VCB i509VCB requested a review from Martmists-GH April 17, 2020 07:21
@i509VCB i509VCB self-assigned this Apr 17, 2020
import net.minecraft.server.command.CommandSource;
import net.minecraft.text.LiteralText;

public class EnumArgumentType<E extends Enum<E>> implements ArgumentType<E> {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this needs to go since we don't use it anymore

@@ -1,7 +1,14 @@
Copyright 2019 Martmists
Copyright 2020 FabLabsMC
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we can keep this in FabLabs, then.

targetCompatibility = 1.8

// Elytra version format - notes branch, commit count, and dirty in version string
def branch
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs cleaning up

@@ -0,0 +1,196 @@
<?xml version="1.0"?>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for checkstyle enforcing

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is meant to be merged into Fabric API, then it needs checkstyle enforcing. If it's not meant to be merged into Fabric API, then it shouldn't be here.

zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.2-bin.zip
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why has this been downgraded? Revert.


# Mod Properties
# library_name may also be used for maven publishing
mod_version = 1.0.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Versioning should stay consistent with existing versioning

}

@Override
public GameRules.RuleType<GameRules.IntRule> createBoundedIntType(int defaultValue, int lowerBound, int upperBound, BiConsumer<MinecraftServer, GameRules.IntRule> notifier) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a function overload with createIntType instead

}

@Override
public GameRules.RuleType<GameRules.BooleanRule> createBooleanType(boolean defaultValue, BiConsumer<MinecraftServer, GameRules.BooleanRule> notifier) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these methods should be called createXXXRuleType instead


import net.fabricmc.api.ModInitializer;

public class GameRuleTest implements ModInitializer {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be removed

@@ -0,0 +1,4 @@
accessWidener v1 named
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this file is used for but seems to be just docs, should be removed

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't docs, this is essential. Access Wideners mean we don't need to dance around private classes.

@@ -1,39 +0,0 @@
package com.martmists.libgamerule.compat.libcd;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LibCD tweakers should still be implemented

@LemmaEOF LemmaEOF reopened this Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants