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

MyPlot Support #17

Merged
merged 22 commits into from
May 23, 2023
Merged

MyPlot Support #17

merged 22 commits into from
May 23, 2023

Conversation

ColinHDev
Copy link
Owner

No description provided.

@ColinHDev ColinHDev linked an issue Apr 19, 2022 that may be closed by this pull request
@ColinHDev ColinHDev added Category: Database Related to internal database handling Type: Enhancement Category: Core Related to internals Category: Generation Related to the world generation Status: TODO Category: Resources Related to the resources of the plugin, e.g. the config labels May 30, 2022
@jasonw4331
Copy link
Contributor

jasonw4331 commented Jul 2, 2022

I've done some light work on comparing the calculations between MyPlot and CPlot. It looks like all the X and Z values for any input rasterized coordinates need offset by the road width for the world generation and position calculations to match up, so it should be easy to add a coordinateOffset value to the WorldSettings for the value to be made available wherever necessary.

@ColinHDev
Copy link
Owner Author

ColinHDev commented Jul 3, 2022

Yeah, if I remember correctly, x=0 and z=0 should be the beginning of a plot in MyPlot, while it is the beginning of the road in this plugin.
I would add an originPlugin property or something like that to the WorldSettings class and adapt the calculations in the plot classes depending on the given world. This would not even be needed to be saved in the database, since we could use the following property of a world instance to check the generator and which calculation to choose: https://github.com/pmmp/PocketMine-MP/blob/stable/src/world/World.php#L273

@jasonw4331
Copy link
Contributor

I have a working version using a simple offset variable saved in the WorldSettings. Would you like me to PR to this branch?
https://github.com/jasonwynn10/CPlot/tree/myplot-support

@ColinHDev
Copy link
Owner Author

Sure!
Thank you very much, this looks very good. I like the approach with the offset property, I would have probably hardcoded it in the plot classes, but this one is better.
The only problem is that an instance of the World is now required to get all WorldSettings instead of only its name. This complicates things for the DataProvider class. Because currently once a MyPlot world is added to CPlot's database, the world type would always be set to CPlot's. The awaitWorld() and other methods could either accept a World instance instead of the world name or, against my previous statement, the world type needs to be saved in the database, which I would be more favourably of.

@ColinHDev
Copy link
Owner Author

ColinHDev commented Jul 5, 2022

I like the idea of the plugin being a drop-in replacement, but the data of the plots does also need to be transferred, so that would need to be addressed as well and would require at least having MyPlot's config file existing to get their provider settings.

@jasonw4331
Copy link
Contributor

The largest inconsistency I see currently is the economy system. MyPlot has prices stored on a per-plot basis, but the prices are locked into the configured values for CPlot. Is it preferred to drop the individual data for the CPlot config values?

As for data sync between the different identifier types, all we need to do is check the player's offline save data for the XUID. As long as the server is XUID validated (which PM is by default), we can use the XUID and UUID interchangeably and match them to the player's name as the file name to sync the data.

@ColinHDev
Copy link
Owner Author

Oh, I didn't know such a feature existed. It would be unfortunate if those data would just be lost, although it's not that big of a deal.
It could just be implemented as a feature in this plugin, but I haven't thought about a good implementation yet, I don't want to add something without a good thought about it just for comparability reasons.
(Maybe not only allowing players to sell their plots at a fixed price, but also allowing bids or something like that. But on the other hand: Does that really need to be in this plugin? I am already debating if the economy part should be part of this plugin or moved to another one as I did with the changeable border, wall and biome when claiming a plot, with the CPlotClaimAddon plugin. But this is going out of the scope of this PR and should probably be discussed elsewhere.)

Don't you need to disable xbox auth to use proxies like WaterdogPE? Is the XUID still saved then?
But it shouldn't matter. With a few final changes, the plugin should internally only use an ID which gets associated here. Although this might not be the best approach either, it allows us to identify players by either name like MyPlot, XUID or UUID.

@jasonw4331
Copy link
Contributor

A feature like auctioning plots or any other extended economy functionality can be extracted into its own plugin easily. That may be a good idea to do for setting individual plot prices. The data import for those features could then also be moved into the new plugin.

I believe WaterdogPE and other proxies are supposed to relay the xuid information to the server for validation still. I will do a bit of research to find more info on how that works.

@ColinHDev
Copy link
Owner Author

Yeah, I was thinking about moving the whole economy part to a different plugin, so even the provider classes, etc. But still debating about that.

I did some work towards this today and there should be no longer any direct dependencies on the UUID, XUID or name, apart from basic player verification. So as long as either one of those three options is known, there should be no problems.

@ColinHDev
Copy link
Owner Author

It seems I've got dementia.
A worldType property already existed since January: 9a0c435

@jasonw4331
Copy link
Contributor

In my fork I made use of that property as a world-type differentiator along with the offset value

@ColinHDev
Copy link
Owner Author

I gave this a second thought. I don't want to fragment this plugin into a dozen of smaller pieces.
Yes, the economy part could be moved to its own plugin, but having it in the core won't hurt. So I will finish #22 and #45, then add those /p sell and /p buy commands and then finally start working on this.

@ColinHDev ColinHDev added this to the v1.0.0 milestone Aug 1, 2022
@jasonw4331 jasonw4331 mentioned this pull request Oct 3, 2022
@jasonw4331 jasonw4331 mentioned this pull request Oct 31, 2022
@ColinHDev ColinHDev marked this pull request as ready for review November 1, 2022 10:53
@ColinHDev
Copy link
Owner Author

This one should be ready for testing and can be downloaded at poggit

…nt()

Signed-off-by: ColinHDev <colinheidfeld@gmail.com>
# Conflicts:
#	src/ColinHDev/CPlot/provider/DataProvider.php
@ColinHDev ColinHDev changed the base branch from development to pm5 May 21, 2023 18:59
@ColinHDev
Copy link
Owner Author

So, I did some testing and now no longer noticed any errors with the imported data.
Therefore this gets finally merged!

@ColinHDev ColinHDev merged commit a677069 into pm5 May 23, 2023
@ColinHDev ColinHDev deleted the myplot-support branch May 23, 2023 21:58
This was referenced Jun 11, 2023
@GamilinoMC GamilinoMC mentioned this pull request Jun 13, 2023
@Atara6431 Atara6431 mentioned this pull request Jan 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core Related to internals Category: Database Related to internal database handling Category: Generation Related to the world generation Category: Resources Related to the resources of the plugin, e.g. the config Status: Completed Type: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MyPlot Support
2 participants