search

architectury / architectury-plugin

A simple gradle plugin to enable developing multiplatform mods.
MIT License
56 stars 19 forks source link

Incorrect classpath in eclipse #4

Closed warjort closed 3 years ago

warjort commented 3 years ago

Using ./gradlew eclipse generates the wrong eclipse classpath for the forge subproject.

The following would be an example of the changes needed to the generated forge/.classpath in architectury itself.

- +

i.e. the forge project depends on the generated mcp jar rather than the common project directly

I can't see a trivial way to fix it from this page? https://docs.gradle.org/current/dsl/org.gradle.plugins.ide.eclipse.model.EclipseClasspath.html

shedaniel commented 3 years ago

I actually don’t use eclipse so I don’t know how it works or how to fix it, if you have any other insights, please do share them

warjort commented 3 years ago

How to post xml in this issues page, inline code?

- <classpathentry kind="src" path="/common"/>
+ <classpathentry kind="lib" path="/common/build/libs/architectury-1.1.9999-mcp.jar" sourcepath="/common/src/main/java"/>
shedaniel commented 3 years ago

seems about right, you need to run common:build to update the mcp remapped version of the common module

warjort commented 3 years ago

https://docs.gradle.org/current/javadoc/org/gradle/plugins/ide/eclipse/model/Classpath.html

I imagine you would have to use the above api to hack the ClasspathEntry(s) in one of the closures of the eclipse plugin configuration?

shedaniel commented 3 years ago

What is wrong with the cp currently?

warjort commented 3 years ago

As it stands the forge project depends upon the common classes compiled over the fabric version of MC. But uses the MCP minecraft classes for its compile.

e.g. for architectury the ToolTipFlag class does not exist in the forge version but it is used implicitly in EventHandlerImplClient::event(ItemToolTipEvent) if you compile directly over common instead of using common-mcp.jar

shedaniel commented 3 years ago

the forge module should use the common-mcp as classpath, and that jar has mcp class names

warjort commented 3 years ago

Yes, that is what the above change to forge/.classpath does.

But changing this manually everytime you change project dependencies (including version changes) will be a pain.

warjort commented 3 years ago

I got some time to dig around to try to find the root cause and the eclipse plugin has some logic to not add the original project if a resolved artefact is a jar. see visitProjectDependency [here:] (https://github.com/gradle/gradle/blob/master/subprojects/ide/src/main/java/org/gradle/plugins/ide/eclipse/model/internal/EclipseDependenciesCreator.java)

Beyond that I got lost in the spaghetti that is gradle, loom, etc. trying to work out what might not be setting the library element attribute properly. Or it might just be that the 2 references to the /common project in the dependencies confuses it?

warjort commented 3 years ago

Anyway adding the following to forge/build.gradle fixes the problem for the architectury project

import org.gradle.plugins.ide.eclipse.model.Library
eclipse {
    classpath {
        file {
            whenMerged {
                entries.removeAll { it.path == '/common' }
                def commonmcp = new Library(fileReference(file('../common/build/libs/architectury-' + project.version + '-mcp.jar')))
                commonmcp.sourcePath = fileReference(file('../common/src/main/java'))
                entries += commonmcp
            } 
        }
    }
}

This feels very hacky, but it does work.

shedaniel commented 3 years ago

It seems like eclipse fails to distinguish between runtime and compile only dependencies

shedaniel commented 3 years ago

We are not supporting eclipse in the near future, I am adding a wontfix tag.

warjort commented 3 years ago

This issue became kind of irrelvant when using the same mappings for fabric and forge. At least I haven't had a problem with it.

Why is eclipse no longer supported? You know visual studio uses the same code for gradle integration as well?

Juuxel commented 3 years ago

Minecraft run configs are created differently on each platform, and Arch Loom needs some extra run config features compared to upstream Loom, which are currently only implemented for IDEA and VS Code run configs. (There might also be something else, but this is one of the main issues.)

warjort commented 3 years ago

Ok, thanks for the info. I personally don't use the run configs, so I think you could say eclipse only has limited support?

Anyway, I will close this issue.