Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ pluginManagement {
}
}

include 'textile-api-base'
include("textile-api-gui")
include 'textile-lifecycle-events-v1'
include("textile-recipe-api-v1")
include('textile-api-base')
include('textile-api-gui')
include('textile-lifecycle-events-v1')
include('textile-recipe-api-v1')
include('textile-utils-api-v1')
10 changes: 10 additions & 0 deletions textile-utils-api-v1/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
archivesBaseName = 'textile-utils-api-v1'
version = '1.0.0'

dependencies {
implementation project(':textile-api-base')
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is unnecessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

actually keep this but move Identifier to api base

}

minecraft {
accessWidener "src/main/resources/textile-utils-api-v1.accessWidener"
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

Copy link

@amusingimpala75 amusingimpala75 Dec 3, 2020

Choose a reason for hiding this comment

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

Why? We need to open up Block.id, Item.id and Block.BLOCKS from being final

Copy link
Contributor

Choose a reason for hiding this comment

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

doesnt need an access widener

Choose a reason for hiding this comment

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

I’m afraid I still don’t understand what you mean. Do you mean 1) that what is being widened is unnecessary, 2) that although it is needed for compilation it is not needed in production, or 3) something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

it is not necessary to use an access widener here. accessors will suffice

Choose a reason for hiding this comment

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

Accessors can modify final variables? I had forgotten that. Okay that makes sense then.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep. Adding a @Mutable does the trick

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package net.textilemc.textile.utils.Block;

import net.textilemc.textile.utils.util.Identifier;

public interface CustomBlock {
Identifier getStringId();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package net.textilemc.textile.utils;

public class Pair<T, M> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn’t guava have Pair?

Copy link
Contributor

Choose a reason for hiding this comment

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

If not this should probably go in api base

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Guava isn't an included library, so we can decide to keep our own, or use Guava here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Loader uses guava iirc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huh? if it does, then could you ready up a commit for using that Pair instead?

private final T left;
private final M right;
public Pair(T left, M right) {
this.left = left;
this.right = right;
}
public T getLeft() {
return left;
}
public M getRight() {
return right;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package net.textilemc.textile.utils;

import net.fabricmc.api.ModInitializer;
import net.minecraft.block.Block;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class RegistrationManagerEntrypoint implements ModInitializer {

public static Logger LOGGER = LogManager.getLogger("Textile API");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public static Logger LOGGER = LogManager.getLogger("Textile API");
public static Logger LOGGER = LogManager.getLogger("Textile Utils API");


@Override
public void onInitialize () {
LOGGER.info("Block registrar - Loading up!");
}

// Cannot return 0 or else MC will hang on the TitleScreen
public static int getNextBlockId() {
for (int i = 1; i < Block.BLOCKS.length; i++) {
if (Block.BLOCKS[i] == null)
return i;
}
return -1;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package net.textilemc.textile.utils;

import net.minecraft.block.Block;
import net.minecraft.block.Material;
import net.minecraft.item.BlockItem;
import net.minecraft.item.Item;

public class SidedRegistry {

public static Pair<Block, Item> registerBlockWithItem(int id, Material material) {
Block BLOCK = new Block(id, material);
if (Block.BLOCKS[id] != null) {
Item.ITEMS[id] = new BlockItem(id - 256);
}
return new Pair<>(BLOCK, Item.ITEMS[id]);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package net.textilemc.textile.utils.mixin;

import net.minecraft.block.Block;
import net.minecraft.client.MinecraftClient;
import net.minecraft.item.Item;
import net.minecraft.nbt.CompoundTag;
import net.minecraft.util.Util;
import net.textilemc.textile.utils.Block.CustomBlock;
import net.textilemc.textile.utils.util.Identifier;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.asm.mixin.Unique;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;

@Mixin(MinecraftClient.class)
public abstract class BlockCacherMixin {

@Shadow
private static File mcDir;

@Unique
private String name;

//Before you ask, yes, caching is necessary in this case. It fails otherwise complaining about "World cannot be cast to String"
@Inject(at=@At("HEAD"), method = "joinWorld(Ljava/lang/String;)V")
public void cacheName(String name, CallbackInfo info) {
this.name = name;
}

@Inject(at=@At(value = "INVOKE", target = "Lnet/minecraft/client/MinecraftClient;joinWorld(Lnet/minecraft/world/World;Ljava/lang/String;)V", ordinal = 1), method = "joinWorld(Ljava/lang/String;)V")
public void removeConflictOlderVersion(CallbackInfo info) {
mapBlockToDat(new File(new File(mcDir, "saves"), name));
Copy link
Contributor

Choose a reason for hiding this comment

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

use nio
File is gonna block the thread

Copy link
Contributor

Choose a reason for hiding this comment

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

mcDir.toPath().resolve("saves").resolve(name)


}
@Inject(at=@At(value = "INVOKE", target = "Lnet/minecraft/client/MinecraftClient;joinWorld(Lnet/minecraft/world/World;Ljava/lang/String;)V", ordinal = 2), method = "joinWorld(Ljava/lang/String;)V")
public void removeConflictOlderVersion2(CallbackInfo info) {
mapBlockToDat(new File(new File(mcDir, "saves"), name));
Copy link
Contributor

Choose a reason for hiding this comment

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

mcDir.toPath().resolve("saves").resolve(name)

}

@Unique
private void mapBlockToDat(File saveDir) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Path saveDir

if (!(new File(saveDir, "block_registry.dat")).exists()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

!Files.exists(saveDir.resolve("block_registry.dat"))

System.out.println("Saving Block Registry to .minecraft/saves/" + name + "/block_registry.dat");
Copy link
Contributor

Choose a reason for hiding this comment

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

use LOGGER.debug here

Copy link
Contributor

Choose a reason for hiding this comment

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

If LOGGER doesn't exist (which it probably doesn't), just create a @Unique private static final Logger LOGGER. Also please (as the above comment says) use paths instead of File objects...

CompoundTag tag = new CompoundTag();
for (int i = 0; i < Block.BLOCKS.length; i++) {
/*if (Block.BLOCKS[i] == null) {
} else */if (!(Block.BLOCKS[i] instanceof CustomBlock)) {
CompoundTag subTag = new CompoundTag();
subTag.putString("name", "$vanilla$");
subTag.putByte("id", (byte) i);
tag.put("block" + i, subTag);
} else if (Block.BLOCKS[i] instanceof CustomBlock) {
CompoundTag subtag = new CompoundTag();
subtag.putString("name", "");
Identifier stringId = ((CustomBlock) Block.BLOCKS[i]).getStringId();
stringId.toTag(subtag);
subtag.putByte("id", (byte) i);
tag.put("block" + i, subtag);
}
}
try {
Util.writeCompressed(tag, new FileOutputStream(new File(saveDir, "block_registry.dat")));
Copy link
Contributor

Choose a reason for hiding this comment

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

Files.newOutputStream(saveDir.resolve("block_registry.dat")

} catch (FileNotFoundException e) {
e.printStackTrace();
throw new NullPointerException();
}
} else {
System.out.println("Matching old Block Registry read from .minecraft/saves/"+name+"/block_registry.dat");
CompoundTag tagFromFile;
try {
tagFromFile = Util.readCompressed(new FileInputStream(new File(saveDir, "block_registry.dat")));
Copy link
Contributor

Choose a reason for hiding this comment

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

Files.newInputStream(saveDir.resolve("block_registry.dat")

} catch (FileNotFoundException e) {
e.printStackTrace();
throw new NullPointerException();
}
Block[] temp = new Block[256];
Block[] finalArray = new Block[256];
Item[] tempItem = new Item[1024];
Item[] finalItem = new Item[1024];
System.arraycopy(Item.ITEMS, 0, tempItem, 0, tempItem.length);
System.arraycopy(Block.BLOCKS, 0, temp, 0, temp.length);
for (int i = 0; i < 256; i++) {
CompoundTag blockEntry = tagFromFile.getCompound("block"+i);
Identifier identifier;
if (!blockEntry.getString("name").equals("")) {
//name = blockEntry.getString("name");
finalArray[i] = temp[i];
finalItem[i] = tempItem[i];
} else {
identifier = Identifier.fromTag(blockEntry);
int id = blockEntry.getByte("id");
int currentId = temp[getCurrentIdFromBlock(identifier)].id;
finalArray[i] = temp[currentId];
finalArray[i].id = id;
finalItem[i] = tempItem[currentId];
finalItem[i].id = id;
}

}
Block.BLOCKS = finalArray;
Item.ITEMS = finalItem;
}
}

@Unique
private static int getCurrentIdFromBlock(Identifier id) {
for (int i = 0; i < 256; i++) {
if (Block.BLOCKS[i] instanceof CustomBlock) {
if ((((CustomBlock) Block.BLOCKS[i]).getStringId()).equals(id)) {
return i;
}
}
}
throw new NullPointerException("Could not find block! You should only see this if you removed the mod which it was in, in which case your world will have corrupted!! Please make a new World!!");
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps don't throw an NPE, instead send a message to the player on world load to tell the player that a block may be missing?

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package net.textilemc.textile.utils.util;

import net.minecraft.nbt.CompoundTag;

public class Identifier {
private final String namespace;
private final String path;


public Identifier (String namespace, String path) {
this.namespace = namespace;
this.path = path;
}

public String getNamespace() {
return namespace;
}

public String getPath() {
return path;
}

public CompoundTag toTag(CompoundTag tag) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You don’t need a CompoundTag for this. That just wastes bytes
Store it as a StringTag

Choose a reason for hiding this comment

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

What’s the use case of CompoundTag vs Tag?

Copy link
Contributor

Choose a reason for hiding this comment

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

CompoundTag is a maplike tag
Tag is the base class for tags

CompoundTag subtag = new CompoundTag();
subtag.putString("namespace", this.getNamespace());
subtag.putString("path", this.getPath());
tag.put("nameId", subtag);
return tag;
Comment on lines +23 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public CompoundTag toTag(CompoundTag tag) {
CompoundTag subtag = new CompoundTag();
subtag.putString("namespace", this.getNamespace());
subtag.putString("path", this.getPath());
tag.put("nameId", subtag);
return tag;
public CompoundTag toTag(CompoundTag tag) {
tag.putString("nameId", this.getNamespace() + ':' + this.getPath());
return tag;

}

public static Identifier fromTag(CompoundTag tag) {
return new Identifier(tag.getString("namespace"), tag.getString("path"));
}
Comment on lines +31 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the toTag method just above, you save this to a nameId entry. You should then retrieve the identifier the same way.

Suggested change
public static Identifier fromTag(CompoundTag tag) {
return new Identifier(tag.getString("namespace"), tag.getString("path"));
}
public static Identifier fromTag(CompoundTag tag) {
String[] value = tag.getString("nameId").split(":");
return new Identifier(value[0], value[1]);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

serialization should not be done directly to a compound tag. nameId sounds weird.

Copy link
Contributor

Choose a reason for hiding this comment

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

in fact, the compound tags in fromTag and toTag are meant to be of that object, so the only data you read is the data you’ve serialized

Copy link
Contributor

Choose a reason for hiding this comment

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

and since Identifiers don’t use compound tags anymore, this should just be a toString and a tryParse

Copy link
Contributor

Choose a reason for hiding this comment

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

(so that the caller can choose any key name and so as to not violate the method contract)

}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
28 changes: 28 additions & 0 deletions textile-utils-api-v1/src/main/resources/fabric.mod.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"schemaVersion": 1,
"id": "textile-utils-api-v1",
"version": "${version}",
"name": "Textile Utils API (v1)",
"description": "Infdev Utils",
"authors": [
"TextileMC"
],
"contact": {
"issues": "https://github.com/TextileMC/textile-api/issues",
"sources": "https://github.com/TextileMC/textile-api"
},
"entrypoints": {
"main": ["net.textilemc.textile.utils.RegistrationManagerEntrypoint"]
},
"license": "CC0-1.0",
"icon": "assets/textile-utils-api-v1/icon.png",
"environment": "*",
"accessWidener": "textile-utils-api-v1.accessWidener",
"mixins": [
"textile-utils-api-v1.mixins.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn’t have any mixins

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, need to remove that as said before, leftover from copying module.

],
"depends": {
"fabricloader": "*",
"minecraft": "20100618"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
accessWidener v1 named
## Blocks
accessible method net/minecraft/block/Block <init> (ILnet/minecraft/block/Material;)V
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary access widener

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That AW is necessary for Pair to function.

Choose a reason for hiding this comment

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

Really? It wasn't when I used it

Choose a reason for hiding this comment

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

Oh yeah, I was using a custom block class

Copy link
Contributor Author

@PhoenixVX PhoenixVX Nov 30, 2020

Choose a reason for hiding this comment

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

That's because you instantiated your Block (Stone) class, I had to instantiate it as a Block as I don't want to hardcode it to be "Stone" or whatnot.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is a Block access widener necessary

Copy link

@amusingimpala75 amusingimpala75 Nov 30, 2020

Choose a reason for hiding this comment

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

Perhaps in the future write a more extensive api that has all modded blocks be instances of a FabricBlock, and thus all with same numerical id, but also have a unique String id, which is checked every time the numerical id is checked, but that would require many mixins to fix that, and that is probably later down the rodd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Look at the top issue. I commented about some ways to do that.

mutable field net/minecraft/block/Block id I
mutable field net/minecraft/item/Item id I
mutable field net/minecraft/block/Block BLOCKS [Lnet/minecraft/block/Block;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"required": true,
"package": "net.textilemc.textile.utils.mixin",
"compatibilityLevel": "JAVA_8",
"mixins": [
"BlockCacherMixin"
],
"injectors": {
"defaultRequire": 1
}
}