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

Remove PsuedoServer #552

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove PsuedoServer #552

wants to merge 1 commit into from

Conversation

okx-code
Copy link
Contributor

It's annoying to maintain and not actually necessary

It's annoying to maintain and not actually necessary
Copy link
Contributor

badge

Build Successful! You can find a link to the downloadable artifact below.

Name Link
Commit 3f20384
Logs https://github.com/CivMC/Civ/actions/runs/9671506379
Download https://github.com/CivMC/Civ/suites/25315259052/artifacts/

@Protonull
Copy link
Contributor

It's only "not actually necessary" because the tests that need it have been commented out. If you uncomment them, you'll see why it's necessary, but PseudoServer actually needs to be updated, which is why drafted #393.

@Protonull
Copy link
Contributor

Perhaps a better solution would be moving it over to the test source set. It's otherwise just taking up space in the production jar when nothing other than the [commented out] tests need it.

@okx-code
Copy link
Contributor Author

I'm not extending internal Bukkit classes

@okx-code
Copy link
Contributor Author

okx-code commented Jun 26, 2024

FakePlayer from HiddenOre also needs to go but it's a mess to keep it while also keeping cave ore generation a thing

@Protonull
Copy link
Contributor

I'm not extending internal Bukkit classes

Yeah, PseudoServer has the Minestom approach [of only adding what you need], whereas extending CraftServer would be the PaperMC approach [of disabling what you don't want]. PseudoServer was added back in the 1.16.4 days (before even the move to Gradle) to avoid spinning up a whole server (and figuring out where the server files should go and when they should be cleaned up) just to test some item stuff. Now that we have :runServer, this is far less of an issue.

@okx-code
Copy link
Contributor Author

mockbukkit btw

@Protonull
Copy link
Contributor

btw, mockbukkit != craftbukkit

@Protonull
Copy link
Contributor

If you intend to go through with this, you should probably remove junit and the test source set altogether too. We simply do not have well any enough managed or organised projects for it, and the stuff we need to test for, like item matching, requires a minimum level of server bootstrapping. It would probably be better to move any test stuff to civmodcore-test with a custom run command that starts a short-lived server instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

None yet

2 participants