[1.16.1] Apply advancements async#4057
Conversation
|
Why isn't async advancements setting turned on by default? , because in my opinion it should |
Because by default every setting should replicate vanilla behavior (or shouldn't, but that's how it is in upstream projects so paper follows) and as this is not vanilla's default behavior in order to get it we disable it by default. |
No. Default settings in Paper are what people would most likely prefer.
Because I just typed out the setting while not really thinking over defaults. |
It needs to be default on because its something that not a large amount of server owners will understand. I know a lot of server owners who wouldn't even know what asynchronous means. By making it default it means those server owners get the performance advantage of computing more tasks asynchronously without having to learn the difference between a task being used by the main thread or not. Paper should set its default options to "what people would most likely prefer," and I think that this option being set to default is just that. |
Not exactly true but sure |
That is literally quoting a project lead. |
aikar
left a comment
aikar
left a comment
There was a problem hiding this comment.
You got it :) this is exactly what I was envisioning. Outside of the few adjustments, I see nothing here that will change game behavior.
So, there's no reason for us to even have a config then, as we only add configs for optimizations if it comes with a con, and i'm not seeing one here.
@MrIvanPlays What Proxi said is correct. That was a quote from me. |
|
I've amended the PR with the changes requested, and also removed some unnecessary null checks (thanks for the |
|
what is the status on this? |
|
Waiting for 1.16.2, we can't merge prs until it's in master |
|
I can rebase it onto 1.16.2 and open a PR if you'd like? |
|
You can, if you want to rebase it on 1.16.2, it would be useful for those who merge it, at least so i think. You can correct me if im wrong in the comments |
|
rebasing is bad for the git history, and only means that there is double the merging effort |
|
I'm somewhat disappointed I have to clarify I mean making a new PR for 1.16.2.
(Assuming it isn't merged into 1.16.2 branch first,) once it becomes master, this will be fixed for merging into master anyways.
August 19, 2020 9:40 PM, "Shane Freeder" <notifications@github.com (mailto:notifications@github.com?to=%22Shane%20Freeder%22%20<notifications@github.com>)> wrote:
rebasing is bad for the git history, and only means that there is double the merging effort
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#4057 (comment)), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AEXQ6M4I6VDEFOFL23TI23LSBQTBPANCNFSM4PUK3WCA).
|
|
I misread the last comment, I don't think there'd be an issue rebasing onto .2, too early I thought they meant master |
7 participants
Asynchronous advancements, take 3.
Advancements are loaded in the same way as before, except asynchronously.
This uses the approach from take 1 (#3454), except with the notes from Aikar
taken into account.
A player with all advancements took an estimated 15-30ms or so every login
on my own machine. The first login of which always took well over 200ms,
hanging around at about 300ms. I'll take the stance that this change will
provide favourable performance alleviations for servers.
I've again taken the approach of a "get if loaded" method in certain cases,
mostly just those which save data. This should lead to corrupt data or simply
not yet loaded data not being saved which results in no data loss and no
waiting for the loading to finish.