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.

Sign up for GitHub

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

Fake mod containers #192

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Fake mod containers #192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e064551
set up a simple fake-mod system
LemmaEOF Jan 6, 2020
8c61932
add isModLoaded support for fake mods
LemmaEOF Jan 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/main/java/net/fabricmc/api/FakeModProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package net.fabricmc.api;

import net.fabricmc.loader.api.ModContainer;
Copy link
Contributor

Choose a reason for hiding this comment

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

There is currently not way to distinguish between a normal and fake mod container.
First I think we should add a isFake/isProvided method.

I think we should have return a ProvidedModContainer which extends ModContainer and a ModContainer getProvider(); so we could track which mod made the fake mod. Of course this would require a way to get what ModContainer provides an entrypoint (so we have a source of the FakeModProvider).


import java.util.Collection;

/**
* Hook for adding "fake" mod containers to Loader's list.
* Should run <i>after</i> {@link ModInitializer#onInitialize()}
*/
public interface FakeModProvider {
Collection<ModContainer> getFakeMods();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be like below instead?

void registerFakeMods(Consumer<ProvidedModContainer> acceptor);

That would allow for a few things:
No need to worry about the whole collection being passed as null.
Anyone using the method would do the following for example:

public void registerFakeMods(Consumer<ProvidedModContainer> acceptor) {
    ProvidedModContainer container1 = ...
    ProvidedModContainer container2 = ...
    acceptor.accept(container1);
    acceptor.accept(container2);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that might be a good idea. I'll think about it.

}
16 changes: 15 additions & 1 deletion src/main/java/net/fabricmc/loader/FabricLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ public class FabricLoader implements net.fabricmc.loader.api.FabricLoader {

protected final Map<String, ModContainer> modMap = new HashMap<>();
protected List<ModContainer> mods = new ArrayList<>();
protected final Map<String, net.fabricmc.loader.api.ModContainer> fakeModMap = new HashMap<>();
protected List<net.fabricmc.loader.api.ModContainer> fakeMods = new ArrayList<>();

private final Map<String, LanguageAdapter> adapterMap = new HashMap<>();
private final EntrypointStorage entrypointStorage = new EntrypointStorage();
Expand Down Expand Up @@ -242,9 +244,14 @@ public Collection<net.fabricmc.loader.api.ModContainer> getAllMods() {
return Collections.unmodifiableList(mods);
}

@Override
public Collection<net.fabricmc.loader.api.ModContainer> getFakeMods() {
return Collections.unmodifiableList(mods);
}

@Override
public boolean isModLoaded(String id) {
return modMap.containsKey(id);
return modMap.containsKey(id) || fakeModMap.containsKey(id);
Copy link
Contributor

Choose a reason for hiding this comment

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

My request for this would require a contract to be established that a IllegalArgumentException would be thrown if two ModContainers of the same modid were to be registered during the registration process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh that's a good point.

}

@Override
Expand Down Expand Up @@ -416,6 +423,13 @@ public void prepareModInit(File newRunDir, Object gameInstance) {
}
}

public void appendFakeMods(Collection<net.fabricmc.loader.api.ModContainer> mods) {
for (net.fabricmc.loader.api.ModContainer mod : mods) {
fakeModMap.put(mod.getMetadata().getId(), mod);
fakeMods.add(mod);
}
}

public Logger getLogger() {
return LOGGER;
}
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/net/fabricmc/loader/api/FabricLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ static FabricLoader getInstance() {
*/
Collection<ModContainer> getAllMods();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should getAllMods() return every single mod, regardless of weather it's fake or real and add a getRealMods() method.

Or should we make this exclusively real mods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like it'd be better to have near-total separation between fake and real mods. I might even want to have isModLoaded have a counterpart isFakeModLoaded instead of the || behavior it has in the current state of the PR.

Copy link
Contributor

@Pyrofab Pyrofab Jan 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I disagree with boundary here, I think mods should be all equal by default. If someone provides a "fake" mod with the same id as a real mod, there should be a good reason, and they should be treated the same.


/**
* Gets all mod containers added through {@link net.fabricmc.api.FakeModProvider}s.
* @return A collection of all added fake mod containers.
*/
Collection<ModContainer> getFakeMods();

/**
* Checks if a mod with a given ID is loaded.
* @param id The ID of the mod, as defined in fabric.mod.json.
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/net/fabricmc/loader/entrypoint/minecraft/hooks/EntrypointClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package net.fabricmc.loader.entrypoint.minecraft.hooks;

import net.fabricmc.api.ClientModInitializer;
import net.fabricmc.api.FakeModProvider;
import net.fabricmc.api.ModInitializer;
import net.fabricmc.loader.FabricLoader;

Expand All @@ -30,6 +31,7 @@ public static void start(File runDir, Object gameInstance) {

FabricLoader.INSTANCE.prepareModInit(runDir, gameInstance);
EntrypointUtils.invoke("main", ModInitializer.class, ModInitializer::onInitialize);
FabricLoader.INSTANCE.getEntrypoints("fake_mods", FakeModProvider.class).forEach(provider -> FabricLoader.INSTANCE.appendFakeMods(provider.getFakeMods()));
Copy link
Contributor

Choose a reason for hiding this comment

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

The question I have is, is this the right spot for the FakeModProviders to run?

I personally think that we should load fake mods after the GameProvider is set but before onInitialize after each of the ModInitializers fires their constructors.
One key reason for doing this before Client/Common/Server entrypoints is a concern about mod loading order, where if you do this after the Common Initializer has gone off you don't exactly have much context in what all mods that exist at that point along with breaking the expectation of the ModInitalizers having a final state for mods being loaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main worry there is that then the fake mods likely haven't been loaded. You'd almost definitely want to do that in onInitialize.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is your reason for loading fake mods after the mod initializer (thereby the client/server would be loaded). Sponge's events cover some states near what on Initialize would cover.

EntrypointUtils.invoke("client", ClientModInitializer.class, ClientModInitializer::onInitializeClient);
}
}
2 changes: 2 additions & 0 deletions src/main/java/net/fabricmc/loader/entrypoint/minecraft/hooks/EntrypointServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package net.fabricmc.loader.entrypoint.minecraft.hooks;

import net.fabricmc.api.DedicatedServerModInitializer;
import net.fabricmc.api.FakeModProvider;
import net.fabricmc.api.ModInitializer;
import net.fabricmc.loader.FabricLoader;

Expand All @@ -30,6 +31,7 @@ public static void start(File runDir, Object gameInstance) {

FabricLoader.INSTANCE.prepareModInit(runDir, gameInstance);
EntrypointUtils.invoke("main", ModInitializer.class, ModInitializer::onInitialize);
FabricLoader.INSTANCE.getEntrypoints("fake_mods", FakeModProvider.class).forEach(provider -> FabricLoader.INSTANCE.appendFakeMods(provider.getFakeMods()));
EntrypointUtils.invoke("server", DedicatedServerModInitializer.class, DedicatedServerModInitializer::onInitializeServer);
}
}