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

Jump to bottom

feat: Minestom platform #485

Open
wants to merge 25 commits into
base: ver/6.6.0
Choose a base branch
from
Open

feat: Minestom platform #485

wants to merge 25 commits into from

Conversation

Bloeckchengrafik
Copy link

@Bloeckchengrafik Bloeckchengrafik commented Dec 29, 2024 •
edited
Loading

Pull Request

Description

This PR implements the terra platform for minestom. You can read more about how this port works on my blog or by looking at the code :)

I just want to get this out here even if it is not complete to get some early feedback on my code. So if you think I should do something different, please let me know!

Closes #113

Changelog

  • Add support for Minestom

Checklist

Mandatory checks

  • The base branch of this PR is an unreleased version branch (that has a ver/ prefix)
    or is a branch that is intended to be merged into a version branch.
  • There are no already existing PRs that provide the same changes.
  • The PR is within the scope of Terra (i.e. is something a configurable terrain generator should be doing).
  • Changes follow the code style for this project.
  • I have read the CONTRIBUTING.md
    document in the root of the git repository.

Types of changes

  • Bug Fix
  • Build system
  • Documentation
  • New Feature
  • Performance
  • Refactoring
  • Repository
  • Revert
  • Style
  • Tests
  • Translation

Compatibility

  • Introduces a breaking change
  • Introduces new functionality in a backwards compatible way.
  • Introduces bug fixes

Documentation

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

I'll add docs to the TerraDocs-Repo when this is done

Testing

  • I have added tests to cover my changes.
  • All new and existing tests passed.

Licensing

  • I am the original author of this code, and I am willing to
    release it under GPLv3.
  • I am not the original author of this code, but it is in public domain or
    released under GPLv3 or a compatible license.

@Bloeckchengrafik Bloeckchengrafik marked this pull request as draft December 29, 2024 19:43
@Bloeckchengrafik Bloeckchengrafik marked this pull request as ready for review January 2, 2025 22:44
@Bloeckchengrafik
Copy link
Author

Bloeckchengrafik commented Jan 2, 2025 •
edited
Loading

I'm done here for now! Please leave a code review and try it out!
Using ./gradlew :platforms:minestom:example:run, you can start a test server and with /regenerate you can
regenerate the world.

If you have any questions or notes about my implementation, please let me know here or in discord!

solonovamax
solonovamax requested changes Jan 3, 2025
View reviewed changes
platforms/minestom/src/main/java/com/dfsek/terra/minestom/TerraMinestomExample.java Outdated Show resolved Hide resolved
common/api/src/main/java/com/dfsek/terra/api/block/BlockType.java Outdated Show resolved Hide resolved
platforms/minestom/build.gradle.kts Outdated

tasks.getByName("run").setProperty("workingDir", file("./run"))

addonDir(project.file("./run/terra/addons"), tasks.named("run").get())
Copy link
Member

Choose a reason for hiding this comment

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

afaik somewhere you need to depend on the installAddons task.

see:

Terra/platforms/fabric/build.gradle.kts

Lines 47 to 55 in 0a952cf

tasks {
remapJar {
dependsOn("installAddons")
injectAccessWidener.set(true)
inputFile.set(shadowJar.get().archiveFile)
archiveFileName.set("${rootProject.name.replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.getDefault()) else it.toString() }}-fabric-${project.version}.jar")
}
}

Copy link
Author

Choose a reason for hiding this comment

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

Should be fixed by the latest commit

platforms/minestom/build.gradle.kts Outdated Show resolved Hide resolved
platforms/minestom/src/main/java/com/dfsek/terra/minestom/MinestomPlatform.java Outdated Show resolved Hide resolved
platforms/minestom/src/main/java/com/dfsek/terra/minestom/chunk/GeneratedChunkCache.java
import org.slf4j.LoggerFactory;


public class GeneratedChunkCache {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Author

Choose a reason for hiding this comment

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

Minestom does not implement separate stages between terrain and feature generation as of now. This is needed to store terrain for "ungenerated" chunks for feature generation, since we can't just load them via minestom

platforms/minestom/src/main/java/com/dfsek/terra/minestom/entity/DeferredMinestomEntity.java
import net.minestom.server.coordinate.Pos;


public class DeferredMinestomEntity implements Entity {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Author

Choose a reason for hiding this comment

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

During World generation, no entities can be placed in minestom. This uses the async chunk loading api to create the entities after the world is loaded

...rms/minestom/src/main/java/com/dfsek/terra/minestom/world/MinestomChunkGeneratorWrapper.java Outdated Show resolved Hide resolved
platforms/minestom/src/main/java/com/dfsek/terra/minestom/MinestomPlatform.java Outdated Show resolved Hide resolved
platforms/minestom/src/main/java/com/dfsek/terra/minestom/world/TerraMinestomWorldBuilder.java
import java.util.function.Function;


public class TerraMinestomWorldBuilder {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have this?

Copy link
Author

Choose a reason for hiding this comment

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

The TerraMinestomWorld class has a lot of parameters. Since Minestom is not configured like bukkit, all worlds need to be created through the API. This makes it easier to create a world. See the provided example for more details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@solonovamax solonovamax solonovamax requested changes

@dfsek dfsek Awaiting requested review from dfsek dfsek is a code owner

@duplexsystem duplexsystem Awaiting requested review from duplexsystem duplexsystem is a code owner

@astrsh astrsh Awaiting requested review from astrsh

@justaureus justaureus Awaiting requested review from justaureus justaureus is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Bloeckchengrafik @solonovamax