Jump to content

Choonster

Moderators
  • Posts

    5116
  • Joined

  • Last visited

  • Days Won

    75

Posts posted by Choonster

  1. 19 minutes ago, Sweetmimike said:

    Hello,

    Thank you for your anwsers, I finally succeeded. I have just one more question : 

    Why in the code below, the parameter TOOL is not considered ? Indeed, when I try to generate loot from a cow, it generates beef whereas I added a parameter TOOL with a diamond sword with fire aspect, so It should generate Steak.

    If you look at the loot table for cows, you'll see that it checks whether the entity is on fire to determine whether to drop steak; not whether the tool has Fire Aspect.

  2. The second argument of withExistingParent is the path to a model file to use as a parent, not a texture. For basic block items, the model normally uses the block model as the parent, rather than specifying individual textures.

    I use this helper method in my BlockStateProvider implementation to generate block item models that simply extend the block model. You can see an example of this here.

    On a side note, the DeferredRegister instance should always be created in the same class as it's used in; don't put the DeferredRegister and RegistryObject fields in separate classes.

    • Thanks 1
  3. On 1/25/2021 at 1:47 AM, ChampionAsh5357 said:

    My only comment would be on OpenClientScreenMessage as the Minecraft instance can just be obtained inside the method itself. This is still 'safe' I believe, but we should avoid calling anything that might only be available on the client that is not isolated in a different class. This is my opinion as it still won't be loaded unless on the physical client.

     

    Thanks, that makes sense.

  4. 19 hours ago, ChampionAsh5357 said:

    From what I understand, this is not the correct way to use DistExecutor. For the case where you can't supply a runnable or supplier, DistExecutor#unsafe* should be used instead. This will supply a runnable of what you want to execute (e.g. () -> () -> //Do things). This does not verify nor guarantee that the code is completely safe to access; however, if the runnable executes another method that is isolated in a different class, it is 'safe' since classloading will not occur. So, the proper way to implement the code above is DistExecutor.unsafeRunWhenOn(Dist.CLIENT, () -> () -> ClientScreenManager#openScreen).

     

    Thanks, I think that makes sense.

     

    I've tried to follow this advice and clean up all my DistExecutor code in this commit, does this look correct?

  5. I have a packet that's sent to the client to open a GUI, which I'm using DistExecutor to do.

     

    The packet's handler method does the following:

    DistExecutor.safeRunWhenOn(Dist.CLIENT, () -> ClientOnlyNetworkMethods.openClientScreen(message))

     

    ClientOnlyNetworkMethods.openClientScreen currently looks like this:

    public static DistExecutor.SafeRunnable openClientScreen(final OpenClientScreenMessage message) {
    	return new DistExecutor.SafeRunnable() {
    		@Override
    		public void run() {
    			ClientScreenManager.openScreen(message.getId(), message.getAdditionalData(), Minecraft.getInstance());
    		}
    	};
    }

     

    ClientScreenManager is a client-only class that handles opening the GUI.

     

    As you can see from the code, I need to pass arguments from the packet to the client-only method; which rules out using a method reference as the SafeRunnable implementation.

     

    When I replace the anonymous class implementation of SafeRunnable in ClientOnlyNetworkMethods.openClientScreen with a lambda, DistExecutor.validateSafeReferent throws an "Unsafe Referent usage found in safe referent method" exception. From what I can see, using any non-lambda implementation of SafeReferent simply bypasses the safety checks in validateSafeReferent but doesn't necessarily mean that the code is safe.

     

    The current code with the anonymous class does seem to work on the dedicated server, but is this the correct way to use DistExecutor; or is there a better way to do it?

  6. 44 minutes ago, diesieben07 said:

    This is why getShareTag should be used for the capability data, not some custom system.

    You don't have any control over when and how ItemStacks are sent over the network, getShareTag is the only real place.

     

    Part of the idea with my system was to allow syncing capabilities attached to arbitrary items, not just items that know about their capabilities. What would you recommend for capabilities attached to items from Vanilla or another mod?

  7. I have a system for syncing item capability data that uses ICapabilityListener, as explained here:

     

    I discovered in that thread that this pull request for 1.12.2 back in 2017 partially broke my system by changing Container#detectAndSendChanges to only call IContainerListener#sendSlotContents if a slot's Item, count or share tag has changed; which often won't be the case for capability-only updates.

     

    The change does make sense for Vanilla IContainerListener implementations to reduce unnecessary network traffic, but would it be possible to allow modded IContainerListeners to opt-in to having sendSlotContents called even if the Items, counts and share tags are equal?

  8. On 12/4/2020 at 5:39 AM, Tavi007 said:

    Hey, so tried to implement what you said and I got it mostly working, but there is one last bug remaining. When I open the play inventory, the itemstacks won't get updated immediately. I have to move the itemstack at least once for it to update. Any other container is working as intended tho. If I open a chest, every itemstack has the right information directly.

    Any idea what the reason might be? Could it have to do something with the curios mod, which I'm using while developing?

    Some links of my github:
    https://github.com/Tavi007/ElementalCombat/tree/master/src/main/java/Tavi007/ElementalCombat/capabilities
    https://github.com/Tavi007/ElementalCombat/tree/master/src/main/java/Tavi007/ElementalCombat/network

     

    It looks like Forge patches Container#detectAndSendChanges to only call IContainerListener#sendSlotContents if a slot's Item, count or share tag has changed; which often won't be the case for capability-only updates. I may need to re-evaluate the IContainerListener system to see if there's any way around this.

     

    This change was actually introduced in August 2017 for 1.12.2, six months after I created my system. I thought it was working more recently than that, but I must not have tested it properly.

  9. With my system, each capability type that needs to be synced to the client has several sync-related classes:

    • A single update network message (extending UpdateContainerCapabilityMessage) that syncs the capability data for a single slot of a Container.
    • A bulk update network message (extending BulkUpdateContainerCapabilityMessage) that syncs the capability data for all slots of a Container.
    • A "functions" class containing static methods used by both the single and bulk update messages.
    • A container listener class (extending CapabilityContainerListener) that sends the single/bulk update messages when the Container's contents change.

    A factory function for the container listener is registered with CapabilityContainerListenerManager.registerListenerFactory at startup so that when a player opens a Container, a new listener can be created and added to it.

     

    The network messages have the concept of a "data" class, which is a simple POJO (or even a primitive type like int or long) containing only the data that needs to be synced from the server to the client. The base classes for the messages handle the functionality that's common to all capability types, the message classes for each capability just need to provide functions to do the following:

    • On the server:
      • Convert an instance of the capability handler (e.g. IFluidHandlerItem for a fluid tank) to a data object
      • Encode (write) the data object to the packet buffer
    • On the client:
      • Decode (read) the data object from the packet buffer
      • Apply the data from the data object to the capability handler instance

    These functions could be defined anywhere (they could even be lambdas passed directly to the base class methods), but I keep them as static methods in a "functions" class so they can be shared between the single and bulk messages.

     

    The system might be a bit over-engineered, but it means that I can easily add syncing for a new item capability without having to rewrite all the common syncing logic.

     

    There are several implementations of this in TestMod3 that you could use as examples:

    • Thanks 2
×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.