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

[MINOR ISSUE] Recipe removal on scroll item not being disabled by default #116

Open
Shibva opened this issue May 6, 2023 · 13 comments
Open

Comments

@Shibva
Copy link

Shibva commented May 6, 2023

Describe the bug
Should be self explanitory from the pic

image
the recipe is not disabled when the teleport/info scroll is not disabled; so it leaves a broken recipe that persist
(Details on image: image of recipe displayed by REI)

@B1n-ry
Copy link
Owner

B1n-ry commented May 6, 2023

Ok there's actually an explanation to this
When I first introduced the scroll item, it was togglable if it would be registered or not as an item. By default not. As it still is to this day.
However when the recipe tried to find this item that did not exist, the game threw an error. This would happen every time when the game started. Someone noticed this and disliked it, so I tried removing the error message. What I did was I replaced the result of the recipe (the scroll) with air. That was the easiest way for me to remove the error in the game log. So it's because someone else didn't like that an error popped up in their log, which wouldn't even crash the game, only look bad or something. So that's why this has happened. Not sure if I'll try to fix it. Could always go back to printing that error instead, since it really wouldn't change anything except a few red lines when the game starts, but be fine after that. I'll see what I'll do

@Shibva
Copy link
Author

Shibva commented May 6, 2023

hmm, well its its possible I would have it based on an internal config wtih the file; I think ive seen it done but its a bit IFFY at best

if its possible id do it based on reading an external IF ELSE statement; since the config setting itself requires a game restart to take effect

@B1n-ry
Copy link
Owner

B1n-ry commented May 6, 2023

Wait do you mean that this happens when the scroll item is enabled? Because that is not supposed to happen

@Shibva
Copy link
Author

Shibva commented May 6, 2023

I mean have the recipe appear when its enabled; the recipe is there when its disable thats the issue; sorry

@Shibva
Copy link
Author

Shibva commented May 6, 2023

(also fast respond time holy shit)

@B1n-ry
Copy link
Owner

B1n-ry commented May 6, 2023

image

@B1n-ry
Copy link
Owner

B1n-ry commented May 6, 2023

So to recap... The recipe works as intended, meaning you can see what it should craft when the scroll item is enabled.
But when it's disabled, there's still a recipe crafting "air", and that is what you'd like to be fixed?

@Shibva
Copy link
Author

Shibva commented May 6, 2023

well, yeah; what im saying is that the recipe should not pop up like its non existent if the item being used to make it is not in the registry based on the config option setting DISSABLED.

theres a few ways; you could do it like I described here or take advantaged at the API to hide the recipe (but that would require integration wtih the other recipe systems API (ie, REI, JEI, EMI, etc)

@Shibva
Copy link
Author

Shibva commented May 6, 2023

basicaly easiest option is to have the recipe file used to be skiped over as if it was not there

@unilock
Copy link

unilock commented Aug 18, 2023

There are ways to add recipes dynamically / conditionally through code. See here for an example:
https://fabricmc.net/wiki/tutorial:dynamic_recipe_generation

(ignore the bit about the "Fabric Data Generation API tutorials"; they don't cover this afaik)

@B1n-ry
Copy link
Owner

B1n-ry commented Aug 19, 2023

I might look into it. The thing is that I want the recipe to be configurable with datapacks, which I'm not sure they will be with the example in this link. I will try to do something that works based on this though

@chorbintime
Copy link

this is still broken btw, at least on 1.19.2 fabric

@B1n-ry
Copy link
Owner

B1n-ry commented Apr 27, 2024

Yeah I've kinda moved away from 1.19.2 for the time being due to me rewriting the mod for 1.20, and I feel like moving between two different sets of code doing essentially the same thing would be kinda annoying. Might backport the 1.20 version when I consider it stable, although it would reset every active grave in world loaded when switching to this new version when still on 1.19.2, so I'm not quite decided yet what the best way of updating would be.
Thanks for the update though!

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

4 participants