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

Dungeon Tweaks Compat again #228

Merged
merged 7 commits into from Oct 3, 2018
Merged

Dungeon Tweaks Compat again #228

merged 7 commits into from Oct 3, 2018

Conversation

jredfox
Copy link

@jredfox jredfox commented Sep 25, 2018

this has backwards compatibility with older dungeon tweaks and the core function re-write update of 1.2.5. This code is also cleaner I hope you will merge this. And yes still this is still a soft dependency

Why do you need to register dungeon entries by default now?: it's to instantiate weights that are not 0 for towers. Users are still able to override the weights via config file it's just the weight won't be default. Also my re-write didn't support any specific mob entries only definitions. If you need the method to be taken down I could add support on my side for weights but, the rest needs to happen.

Also if you don't like my nether specific enhancements by default when dungeon tweaks is loaded I could remove that and have users config them to be that or whatever they wanted. Just thought it would be cool.

Thanks for your time

jredfox added 3 commits September 24, 2018 21:09
also with backwards compat
removed to match your current state
Copy link
Owner

@AtomicStryker AtomicStryker left a comment

Choose a reason for hiding this comment

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

How is this a soft dependency if there are flat out import statements? Unless they are API classes you added? Or do i misunderstand something?

*/
public static void registerDungeons()
{
if(isLegacy) return;//I supported this mod in older versions
Copy link
Owner

Choose a reason for hiding this comment

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

Formatting

Copy link
Author

@jredfox jredfox Sep 25, 2018

Choose a reason for hiding this comment

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

yes I will download your formattor again I guess

addDungeonMob.invoke(null,tower.getId(), nether ? new ResourceLocation("wither_skeleton") : new ResourceLocation("skeleton"), 120);
addDungeonMob.invoke(null,tower.getId(), new ResourceLocation("zombie"), 120);

if(nether)
Copy link
Owner

Choose a reason for hiding this comment

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

formatting

import java.util.Random;

import com.evilnotch.dungeontweeks.main.world.worldgen.mobs.DungeonLocation;
import com.evilnotch.dungeontweeks.main.world.worldgen.mobs.DungeonMobs;
Copy link
Owner

Choose a reason for hiding this comment

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

Pretty sure the idea of compat proxies is to avoid having to IMPORT the addons?

Copy link
Author

@jredfox jredfox Sep 25, 2018

Choose a reason for hiding this comment

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

ok I will remove the imports but, I don't use them. I converted them to reflection so you don't have to have my mod to compile even though it would be safe using it without the mod since loader is mod loaded

@jredfox
Copy link
Author

jredfox commented Sep 25, 2018

import statements do not exist in compiled code. I removed those import statements and the dungeon tweaks from the java build path in the last two commits. I forgot to remove it from the java build path and look for invalid imports my bad.

If you are talking about DungeonTweaksCompat.class it's a class attatched directly to the battle towers commit

The dungeon tweaks compat class is pure refelction currently.

jredfox added 2 commits September 25, 2018 16:21
fixed exception cacheing spawner data when dungeon tweaks wasn't loaded optimized Loader.isModLoaded with cached boolean.
so this doesn't happen again
@jredfox
Copy link
Author

jredfox commented Sep 27, 2018

the mod works without dungeon tweaks and compiles without dungoen tweaks I don't see a problem with the pr sorry If I confused you.

@jredfox
Copy link
Author

jredfox commented Oct 3, 2018

Is there still something wrong with this pull? I removed invalid imports reformated code you said was maul-formed and it's pure reflection thus it's a soft dependency. Sorry if your just to busy to take a look and merge. it's just that people want to play battle towers and dungeon tweaks without a crash this pull was to fix both old and new dungeon tweaks.

@AtomicStryker AtomicStryker merged commit 365d9d6 into AtomicStryker:1.12.2 Oct 3, 2018
@AtomicStryker
Copy link
Owner

Sorry, i was busy and reading scattered commits like this isn't easy. I looked over the squashed merge again and apart from a superfluous refactoring you did (adding Random as parameter when all you do is pull it from another parameter World beforehand...) it looks fine. I've updated the curse version with the new build.

I hope you tested the result to be working runtime, because i did not.

@jredfox
Copy link
Author

jredfox commented Oct 4, 2018

Yeah well I thought you were going to attempt to use random as vanilla minecraft does but, I was wrong. You use world.rand which in the same pos will have different stuffs every time on both structures and spawners according to the logic of your mod. So just ignore it. I was able to convert it to vanilla minecraft at least the spawners but, unless the structure itself is vanilla mc style I wouldn't fix it.

vanilla mc logic is same seed same structure same loot same exact structure mobs etc....

@AtomicStryker
Copy link
Owner

Yes, i am not making sure it generates deterministic because i don't care, but if that is a point for you i can understand the reasoning.

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.

None yet

2 participants