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

Added support for several mods #207

Merged
merged 17 commits into from
Oct 6, 2024

Conversation

vizthex123
Copy link

@vizthex123 vizthex123 commented May 24, 2024

  • Bigger Reactors (also renamed the file for Extreme Reactors to make it clearer)
  • Deeper Caves
  • Deep Resonance
  • Galosphere (added Salt Blocks since Silver Ore was already done)
  • The Outer End
  • Project Red
  • Regions Unexplored
  • RF Tools
  • Thermal Series
  • XyCraft: World (only Kivi and Aluminum since Xychorium ores don't copy over textures)
  • Yung's Cave Biomes

- Bigger Reactors (also renamed the file for Extreme Reactors to make it clearer)
- Deeper Caves
- Deep Resonance
- Galosphere (added Salt Blocks since Silver Ore was already done)
- The Outer End
- Regions Unexplored
- RF Tools
- Thermal Series
- XyCraft: World (only Kivi and Aluminum since Xychorium ores don't copy over textures)
@vizthex123
Copy link
Author

I'm hoping the mod dev can figure out why Xychorium variants don't have textures (mentioned it #205). I think it uses a custom shader or something that Dynamic Asset Generator isn't properly detecting.

@lukebemish
Copy link
Member

I'll look into the missing textures; I suspect you're write, they probably have some sort of custom model thingy I can't detect. I'll try to review the rest of this PR when I have a chance, though 1.20.1 is fairly low priority right now so it may be a bit.

@vizthex123
Copy link
Author

Alright, that sounds good.

Just wanted to toss in what I'd done to save you some time (I figured it probably wasn't too hard to release an update for 1.20.1 if code wasn't changed) and support a couple major-ish mods that were missing it.

Added `_ore` to each ID and `orename` to a few that were missing it.
Also blacklisted Kivi Aluminum again since I misunderstood how that worked.

I still can't get Slate to blacklist vanilla ores ffs
@lukebemish
Copy link
Member

Sorry I've been so delayed in looking at this! I should have more free time soon so I'll plan on getting it looked over and merged soon-ish.

@vizthex123
Copy link
Author

vizthex123 commented May 30, 2024

noice, thanks.

If you're able to check why it's making Slate variations of vanilla ores when Deeper Caves is installed it'd be nice. The mod adds them already, and I tried to blacklist the default ores but it's not working for me - finally fixed that in bd4d1cb

If you could check on Xychorium ores not having textures it'd be great. I can open another PR to add them, or add them to this one before it gets merged.

I had an epiphany to just not assign a type to them lol
I really should've triple-checked all of this
before forking the repo cuz now i've got a dozen useless commits lol
@lukebemish
Copy link
Member

lukebemish commented Jul 13, 2024

Once the remaining feedback is addressed I'll get this merged. I can handle porting it forward to newer versions myself; that shouldn't be too bad. Thank you a bunch for contributing!

@vizthex123
Copy link
Author

Got everything resolved, and yeah I don't mind. If I ever find more mods missing support, I'll open another PR.

If you could check on Xychorium and add it in an update alongside this PR it'd be amazing. Would love to give all XyCraft ores support for this mod.

Copy link
Member

@lukebemish lukebemish left a comment

Choose a reason for hiding this comment

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

Still need to fix whatever bunch of random changes happened to that one file...

@vizthex123
Copy link
Author

vizthex123 commented Jul 15, 2024

re-downloaded it off the github.

Apparently I re-organized the file to make it more readable when making the new files I added.
@lukebemish
Copy link
Member

Looks good to me; I'll get it merged when I've got some free time

@vizthex123
Copy link
Author

Alrighty.

I'll add support for Project Red too.

@lukebemish
Copy link
Member

Please do not modify this PR further before I merge it. I'll have to review any modifications you make and that'll set it back. Don't feel bad about splitting stuff into separate PRs

Can't believe I forgot such a classic mod lol
@vizthex123
Copy link
Author

vizthex123 commented Jul 15, 2024

Sorry, already did that before you commented (or at least before I saw you left a comment). Still not used to making a ton of small commits & whatnot (complete opposite of how I do things lol, and I really hate having to make a ton of tiny changes).

I'll keep it in mind for next time though.

I'm annoyed I have to make another damn commit for this. Can't believe I forgot to double-check it ;-;
Copy link
Member

@lukebemish lukebemish left a comment

Choose a reason for hiding this comment

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

Looks good. I'll get it out in the next release -- may be just a bit as I have to get all the stuff for that older version working again and port stuff forwards before the release.

@vizthex123
Copy link
Author

noice, thanks. Looking forward to it.

@lukebemish lukebemish merged commit 1a9350b into lukebemishprojects:1.20.1 Oct 6, 2024
@lukebemish
Copy link
Member

That last commit was (intentionally) reverted and not included when I merged the PR -- if you want to submit it, it should be a separate PR (in fact compat for different mods should be in general) -- adding more stuff after a PR is approved just means that it has to go through the approval process again. The other changes will be released for 1.20.1 as soon as I've got it ported to newer versions (1.21.x) as well.

@vizthex123
Copy link
Author

yeah, i couldn't figure out how to open another one when one was already open (i'm really dumb and only use github for backups lol).

but i'll see if i can do it now.

thanks for merging it.

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

Successfully merging this pull request may close these issues.

2 participants