Skip to content

Comments

docs: Clarify setup() and teardown() usecases#12

Draft
vdaysky wants to merge 2 commits intoMineplex-LLC:masterfrom
vdaysky:master
Draft

docs: Clarify setup() and teardown() usecases#12
vdaysky wants to merge 2 commits intoMineplex-LLC:masterfrom
vdaysky:master

Conversation

@vdaysky
Copy link
Contributor

@vdaysky vdaysky commented Feb 6, 2025

Thanks for contributing to our Studio Documentation!

To ensure quality and consistency, please complete the checklist below. Provide links and details where requested. This helps us verify everything is in order.

Please ignore any options that aren't applicable to your change.

Documentation

  • I have checked grammar and flow to be sure it's simple to understand
  • I have verified that all terms are consistent.
  • I have confirmed all links are accurate and up to date.
  • I have ensured each section includes all necessary details and meets the outlined objectives.
  • I have checked that any images, diagrams, or tables are clear, correctly formatted, and enhance understanding.

Testing

  • Required - I have ran this locally and it works.
  • I have verified functionality across all supported devices and browsers.
  • I have verified that existing features remain unaffected by recent changes.

Once completed, share all Pull Request (PR) links in the #documentation-chat channel for review by other partners and the Mineplex development team. Thank you for your effort in improving our documentation!

Copy link
Contributor

@BillyDotWS BillyDotWS left a comment

Choose a reason for hiding this comment

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

A couple minor changes


<Note type="note">
Registering a module using `MineplexModuleManager` will call it's `setup()` method.
If module is a `org.bukkit.event.Listener`, it will be registered with Bukkit as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

All modules are listeners:
public interface MineplexModule extends Listener, Teardown, Setup

I think it's worth mentioning that they're registered, but let's tidy up the explanation here.

<Note type="note">
Registering a module using `MineplexModuleManager` will call it's `setup()` method.
If module is a `org.bukkit.event.Listener`, it will be registered with Bukkit as well.
Similarly, destroying a module will call its `teardown()` method, also unregistering events.
Copy link
Contributor

Choose a reason for hiding this comment

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

'destroying a module'?

```

<Note type="warning">
Constructing a `GameMechanic` through `MineplexGameMechanicFactory` will not cause Studio Engine to call neither of its `setup()` nor `teardown()` methods.
Copy link
Contributor

Choose a reason for hiding this comment

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

neither of its -> either

```

<Note type="warning">
Constructing a `GameMechanic` through `MineplexGameMechanicFactory` will not cause Studio Engine to call neither of its `setup()` nor `teardown()` methods.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's worth improving the examples on this page too whilst you're here

@BillyDotWS BillyDotWS added the docs: enhancement An improvement or extension of existing documentation label Feb 6, 2025
@BillyDotWS BillyDotWS self-assigned this Feb 6, 2025
@vdaysky
Copy link
Contributor Author

vdaysky commented Feb 7, 2025

I made some changes, wanted to know your opinion on those - if you don't agree I'll just rollback. I feel like this new structure would be more intuitive, and I am saying this as someone who spent a lot of time staring at those docs, trying to understand how things work. I acknowledge that it is possible that you already considered this structure and decided against it, and if so I would be curious to know the reasoning.

I renamed built-in mechanics page into just mechanics, and moved all of the mechanics docs from game engine module page to mechanics page (potentially if you don't like the nesting mechanics page can be moved to uppermost sdk section level for better visibility, but since mechanics are coupled heavily with the game having it as nested page also makes sense. Having it inside game engine page though is too much IMO. Currently that page has information on game cycle, game interface, game states, mechanics, custom mechanics, mechanic factories, has two 175-line listings that have 4 lines of diff between them and it also documents GameWorldSelectorMechanic for some unknown reason). Also weirdly enough path of the mechanics page (/features/game/mechanics) was already implying that it is sub page of game engine module (I am starting to think that it was, but someone changed it while preserving path for backwards compatibility).

Another side effect of moving both custom and built-in mechanics to one page is that same has to be done for modules, and honestly I think that would be a great change. There are two separate pages for built-in and custom modules, while they are really the same thing, with built-in being custom modules implemented and registered by mineplex (also both pages are relatively empty). On an unrelated note, why is that page called "Features" in navbar, while it contains modules and index page is titled as modules?

Implementing your own custom `GameMechanic`s is easy to do: just create a new class that implements the `GameMechanic`
interface, and determine if you want a mechanic that is coupled to a game, for example `BossMechanic<BossGame>`, or a
more generic mechanic, such as `SpectatorMechanic<MineplexGame>`. In your custom class, you will need to implement
the `setup(Game)`, `teardown()`, and `onStateChange(GameState, GameState) - todo: wtf is this` methods to define the custom behavior of your
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is this onStateChange though? Is it something from a super old sdk version?

Copy link
Contributor

Choose a reason for hiding this comment

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

This appears to be removed, likely for handling state changes on older versions

@vdaysky vdaysky marked this pull request as draft February 7, 2025 17:57
@BillyDotWS BillyDotWS removed their assignment Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs: enhancement An improvement or extension of existing documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants