Skip to content

Migrate multithreaded tracker config#292

Closed
encode42 wants to merge 2 commits into
PurpurMC:ver/1.16.5from
encode42:organize/mt-tracker
Closed

Migrate multithreaded tracker config#292
encode42 wants to merge 2 commits into
PurpurMC:ver/1.16.5from
encode42:organize/mt-tracker

Conversation

@encode42

@encode42 encode42 commented Apr 23, 2021

Copy link
Copy Markdown
Contributor

This isn't an Airplane feature anymore and causes confusion for the Airplane support team and Purpur's users.

  • AIR doesn't have a method to set a value as null. We can either:
    • Set section comments to reflect migration [current solution]
    • Utilize an overcomplicated method to remove the section
    • Do nothing [Airplane's solution]
  • Setting the section comment seems to be broken currently. I contacted Paul for support on why it's not working. I'd like to get this fixed before merge.
  • This is a new patch since I don't want to deal with all the merge conflicts placing it after the Purpur config files. Feel free to adjust this if anyone has the time.

@YouHaveTrouble YouHaveTrouble left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@12emin34 12emin34 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@BillyGalbreath

Copy link
Copy Markdown
Contributor

Now you see why I haven't gotten around to doing this myself. To do a proper migration airplane needs a method so I can save a null value to (or remove altogether) these 2 options. Until that exists something like this isn't going to happen. And when it does happen it will be a complete clusterfuck because I'm going to move the entire patch to the end of the list.

On the other hand, I'm not all that pressed because its a very minor QoL change that has no real benefit, especially seeing that the option is disabled by default (i.e., most people wont even see the option exists). Mojang is going to release 1.17 here in the next few months anyways, and this is one of the patches that are being dropped at that time.

I only kept this patch around because airplane decided to drop it entirely, even though it works fine for most servers. It is not my intention to maintain it forever.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants