Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clumps x Inventorio #118

Open
DegIoving opened this issue Apr 21, 2023 · 15 comments
Open

Clumps x Inventorio #118

DegIoving opened this issue Apr 21, 2023 · 15 comments

Comments

@DegIoving
Copy link

So in the modpack im running 1.19.2
I run another mod called Inventorio, where it adds a Toolbelt where you can have your tools and basically a auto-tool feature to it. (and other non relateable features for my issue)

So the issue is, inventorio lets XP repair tools in toolbelt when they have mending,
but after instlalling Clumps, it doesnt seem to mend the tools in toolbelt.

is there any Config to the mod (Clumps) where it might be a setting i can change

@jaredlll08
Copy link
Owner

jaredlll08 commented Apr 21, 2023

Unfortunately this seems like a mod compat issue that I don't see a quick and easy fix for, at-least from my side alone.

If @Lizard-Of-Oz is willing to work together, I can fire an event here https://github.com/jaredlll08/Clumps/blob/1.19/Common/src/main/java/com/blamejared/clumps/mixin/MixinExperienceOrb.java#L126 that they can then listen to and change the value of it to be an item from their toolbelt.

That would solve the issue, beyond I'm not sure there is a simple fix.

@Lizard-Of-Oz
Copy link

When you inject repairPlayerItems, try replacing it with onPlayerCollision instead. This is the method Inventorio is injecting itself into. I think they shouldn't conflict with each other, and it should fix things without any undesired side-effects.

Alternatively, you can call

int xpLeft = InventorioAPI.getInventoryAddon(player).mendToolBeltItems(amount);

The return value of this method is the amount of xp left after mending the toolbelt items.

@jaredlll08
Copy link
Owner

I think you may be misunderstanding my mod, I wholesale replace a lot of the experience orb code to account for it all being clumped together into a single orb (vanilla only clumps orbs of the same experience value, Clumps does any amount of experience value).

So I can't just move the inject as that will not function the same in any way.

I am not interested in calling other mod's code myself, that will end up with me having a lot of if mod loaded, do this checks all over the place and having to keep track of all the mods I have hardcoded support for in-case their mod changes and now I need to update the code to account for that.

That is why I suggested the event based approach, that way it doesn't matter how your code changes, the mods are going to remain compatible.

After further thought, I believe I actually already have an event that you can use.

public class ValueEvent implements IValueEvent {

        ClumpsEvents.VALUE_EVENT.register(event -> {
            int xpLeft = InventorioAPI.getInventoryAddon(event.getPlayer()).mendToolBeltItems(event.getValue);
            event.setValue(xpLeft );
            return null;
        });

@Lizard-Of-Oz
Copy link

Lizard-Of-Oz commented Apr 21, 2023

I'm busy with doing other things right now for the following month.

> I am not interested in calling other mod's code myself

But would your approach require my mod to have hardcoded support for your mod?

@Lizard-Of-Oz
Copy link

Actually, nevermind. I got the idea and will implement the change once I have the time. I see the point that you can't really inject Inventorio code in a neat way.

@jaredlll08
Copy link
Owner

Sounds good, unfortunately yes this will require one of our mods to add support for the other.

If you would like, I can look into making a PR into Inventorio to add the support (I would do it how I suggested above with the event, but if you have another idea, I'm all ears)

@Lizard-Of-Oz
Copy link

I'll figure it out. Will have to change some things in my mod anyway. Thanks for the example.

@Lizard-Of-Oz
Copy link

It seems like VALUE_EVENT provides an invalid value - a clumped orb triggers 1-3 events with values in the range between 1 and 10, even if the clumped orb itself has accumulated over dozens of orbs.

This result in Inventorio's ToolBelt mending not getting nearly as much xp from a clumped orb as it should.

@Lizard-Of-Oz
Copy link

I checked multiple times that the issue is on the event's side: I get far less events with far less total xp than a clumped orb should have (when a clumped orb's xp get applied to the character, it gets the full expected xp)

@Lizard-Of-Oz
Copy link

My theory: the event system prevents from firing too many events at the same time, and it seems like you're firing an event for every orb collected inside the clump.
This bug is present across all supported versions.

@jaredlll08
Copy link
Owner

the event system prevents from firing too many events at the same time

What modloader are you testing on and can you test on the other modloader?

I have never heard of an issue like that before (too many events being fired), but I will investigate and make sure that VALUE_EVENT is getting the correct values

@Lizard-Of-Oz
Copy link

Both Forge and Fabric appear to have this issue it seems like.

@jaredlll08
Copy link
Owner

If it's possible, could you please build a fabric version of your mod using the event so that I can test

@Lizard-Of-Oz
Copy link

inventorio.zip

@jaredlll08
Copy link
Owner

jaredlll08 commented May 25, 2023

Ah okay, I see the issue, it seems that I had forgotten what that event was actually for, which is not this.

I've now added a RepairEvent that should be used instead, it is fired before vanilla items are repaired, so you can repair your items instead, and change the event value to reflect the used experience.

"com.blamejared.clumps:Clumps-common-1.19.4:10.0.0.3"
"com.blamejared.clumps:Clumps-fabric-1.19.4:10.0.0.3"
"com.blamejared.clumps:Clumps-forge-1.19.4:10.0.0.3"

int leftOver = Services.EVENT.fireRepairEvent(player, actualValue)
.map(IRepairEvent::getValue, UnaryOperator.identity());
if(leftOver == actualValue) {
leftOver = this.repairPlayerItems(player, actualValue);
}
if(leftOver > 0) {
toGive.addAndGet(leftOver);
}

ClumpsEvents.REPAIR_EVENT.register(event -> {
            event.setValue(event.getValue());
            return null;
        });

Sorry for the confusion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants