-
Notifications
You must be signed in to change notification settings - Fork 94
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
Ancient cyphers #838
Ancient cyphers #838
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the config system here could use some work, ideally have it be datapackable and not stuffed all into a List. Arguably that could be done later? But I'd rather not have it be a breaking change later with dropping support for this config system.
Otherwise just small changes.
Common/src/main/java/at/petrak/hexcasting/common/casting/actions/spells/OpMakePackagedSpell.kt
Outdated
Show resolved
Hide resolved
Common/src/main/java/at/petrak/hexcasting/common/lib/hex/HexActions.java
Outdated
Show resolved
Hide resolved
Common/src/main/resources/assets/hexcasting/textures/item/scroll_ancient_large.png
Outdated
Show resolved
Hide resolved
Common/src/main/java/at/petrak/hexcasting/common/loot/AddHexToAncientCypherFunc.java
Show resolved
Hide resolved
Common/src/main/java/at/petrak/hexcasting/common/loot/HexLootHandler.java
Outdated
Show resolved
Hide resolved
Forge/src/main/java/at/petrak/hexcasting/forge/ForgeHexConfig.java
Outdated
Show resolved
Hide resolved
Common/src/main/java/at/petrak/hexcasting/common/loot/HexLootHandler.java
Show resolved
Hide resolved
Actually, if we wanna just leave it non configurable and have it be hard-coded for now then we could figure out the datapack support in the future without locking us into this? I'd like to get more datapack infrastructure set up based around Codecs at some point in the future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me. left some small nitpicks. Otherwise fine to merge
Common/src/main/java/at/petrak/hexcasting/common/loot/AddHexToAncientCypherFunc.java
Outdated
Show resolved
Hide resolved
Common/src/main/java/at/petrak/hexcasting/common/loot/AddHexToAncientCypherFunc.java
Outdated
Show resolved
Hide resolved
Forge/src/main/java/at/petrak/hexcasting/forge/ForgeHexConfig.java
Outdated
Show resolved
Hide resolved
Common/src/main/java/at/petrak/hexcasting/api/mod/HexConfig.java
Outdated
Show resolved
Hide resolved
82677db
to
5976e24
Compare
Adds ancient cyphers, as seen in #818. Implements #432.