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

Reintroduce rot related commands #1094

Closed
wants to merge 3 commits into from
Closed

Conversation

Pablete1234
Copy link
Member

Fixes a few missing things on the recent cloud refactor, like aliases for settings (ie: /toggle dms wouldn't work, only /toggle deaths), and re-introduces the removed rot, rots, and setrot commands. Additionally those commands now will, as their name indicates, operate on rots. If any exception happens while running them they'll suggest using the pool-named commands, with a clickable-hover-able text.

@Pablete1234 Pablete1234 added bug Something isn't working reintroduce Reintroduces a feature that was removed labels Nov 5, 2022
@alexsosnovsky
Copy link
Contributor

A significant amount of PGM users are accustomed to utilizing the /rots, /rot, and /setrot commands to manipulate map organizations. This is due to rotations conceptually predating map pools by a large margin of time. Map pools were introduced later to co-exist with rotations, with rotations being absorbed as a type of map pool, and likewise new pool-related commands introduced.

With some opposition (and/or ignorance), the rotation-based commands were removed earlier this year, intending to force users to utilize the pool-based commands. This change was reverted several days ago, with overwhelming support by the PGM community.

Now, the /rot, /setrot, and /rots commands are proposed for modification to filtered versions of the map pool commands, rather than with their original functionality. The filtered pool command conceptually has nothing wrong with it, just there are significant issues with it usurping the legacy commands:

  1. This is a change in functionality from the original implementation, defying user expectations. If a user sends a rotation command, they expect to get all of the map pools. Instead, they may be confused as to why certain ones aren’t showing up. While the message to utilize /pools may help in awareness, it still forces the user to switch commands completely unnecessarily.
  2. This functionality is pretty much useless on servers that have either shuffled or voted map pools, possibly mixed with ordered ones. This change would essentially force users to instead utilize /pools, by making the /rots command not return information that users typically want. Anyone who actually wants the filter is free to use the flags, but the vast majority of users looking to utilize rotation-based commands are not taking that into consideration.
  3. This change is generally not desired by actual users of the commands. While some are neutral with regards to the changes, many would simply prefer the return of the commands as they were, with nothing removed. From the users’ perspective, it is entirely irrelevant that rotations are a subset of pools, this is how the objects are organized in the code, but doesn’t impact the users’ ability to utilize the command successfully.

This PR is the solution to a problem that doesn’t exist. Demand for this filtering from the rotation-based commands has been minimal. I perceive it as simply a method of enforcing the internal code structure upon the end user, without considering consequences.

@KingOfSquares
Copy link
Contributor

It's funny, I want to agree with Pablo, but that just confirms the case of people developing wanting to change vs the people using wanting to keep.

@Pablete1234
Copy link
Member Author

A significant amount of PGM users are accustomed to utilizing the /rots, /rot, and /setrot commands to manipulate map organizations.

I believe very few pgm users to actually use these commands at all, mostly, map developers. You are one of the people impacted by the change and are accustomed to using the command, as well as talking/interacting with others in the same position, which likely makes you biased towards an importance to keeping them intact that does not hold true for most users.
Small note, you can call them map pools instead of "map organizations", it's baffling that you try to avoid even naming them.

This is due to rotations conceptually predating map pools by a large margin of time. Map pools were introduced later to co-exist with rotations, with rotations being absorbed as a type of map pool, and likewise new pool-related commands introduced.

Correct, rotations used to be the only type of pool back in the day, this however changed so long ago, that alternative pool types have existed in PGM for longer than rotations did standalone, with the first iteration of dynamic pools existing shortly after OCC's closure when 1.11 PGM went OS. I don't believe pool commands to have existed back then, but it was the start of changes to the ideas of how the next map should be determined.

With some opposition (and/or ignorance), the rotation-based commands were removed earlier this year, intending to force users to utilize the pool-based commands. This change was reverted several days ago, with overwhelming support by the PGM community.

The rotation-based commands were removed due to their irrelevance in the new meta, where very few people continue to use rotations as their main type of pool, they were a legacy alias with incorrect behavior as it no longer was managing rots, rather, pools. This change was reverted several days ago by the same small (but vocal) minority using these commands, and refusing to change to the new commands that actually represent the operations they're trying to perform (in other words: pool operations, and not rotation operations).

Now, the /rot, /setrot, and /rots commands are proposed for modification to filtered versions of the map pool commands, rather than with their original functionality. The filtered pool command conceptually has nothing wrong with it, just there are significant issues with it usurping the legacy commands:

They are proposed to keep their actually legacy functionality: managing rotations, which is what they used to do when rotations were the only way to play maps. Additionally, they're getting fixes for usability that have been rightly criticized:

  • Confusion on users, not being able to use the command and wondering what happened
    • This has been fixed by the added alternative usages message, that will point them towards the proper command when rotations aren't available or aren't being used.
  • Servers still using rotations not being able to manage them with /rots etc
    • This has also been fixed as the commands will work the same to manipulate rotations, just as they did from the very beginning.

This is a change in functionality from the original implementation, defying user expectations. If a user sends a rotation command, they expect to get all of the map pools.

I do not consider that the intended behavior of a command called rot or rots shorthand for rotation/rotations, if the user wishes to manage all map pools they're able to use the pool command to get all of them. This is intended behavior, and if the current pool isn't a rotation they'll get a nice message pointing them towards the correct command, which will help users learn the new commands instead of being left without a response or with the wrong expectation.

While the message to utilize /pools may help in awareness, it still forces the user to switch commands completely unnecessarily.

I don't consider it unnecessary to expect the user to use the pool command to manage pools, and as mentioned earlier, they'll get a hint with clickable text to point them towards it.

This functionality is pretty much useless on servers that have either shuffled or voted map pools, possibly mixed with ordered ones.

This functionality will still help users by pointing them to the right commands if they're using the legacy ones and it's likely not to be what they want. Additionally, doing things like /setrot will tab-in rotation names instead of pool names, which is a new functionality, giving flexibility for when you actually want to set specifically a rotation (eg: funky friday)

This change is generally not desired by actual users of the commands.

I am trying my best to find a middle-ground that will try to suit all parties involved. Precisely due to the removal not being welcomed it is that i am bringing them back, but trying to do so in a way that will be beneficial to both the plugin and to the users, by informing them about the migration path to the new commands, and making the command fit its naming instead of outright removing it.

Lastly:
If there's any other problems, bugs or unintended side-effect with the functionality i'm happy to hear them and adapt the code, this is still a draft pull request for that reason. However, feedback that simply back-pedals on "i don't want any change at all, and it must all be kept exactly as-is because i do not want to use pool commands to manage pools" won't be taken into consideration.

I hope we can find a middle-ground that will satisfy all parties, for the benefit of PGM, which we all are trying to improve one way or another.

@alexsosnovsky
Copy link
Contributor

alexsosnovsky commented Nov 5, 2022

I believe very few pgm users to actually use these commands at all, mostly, map developers. You are one of the people impacted by the change and are accustomed to using the command, as well as talking/interacting with others in the same position, which likely makes you biased towards an importance to keeping them intact that does not hold true for most users.

This is correct, and makes sense. The users accustomed to using these commands, mostly map developers, are indeed the ones voicing opposition to the commands’ disfiguration. Why would we expect average users to have a strong opinion about the topic, if they don’t use these commands regularly enough to be strongly affected? Now, this isn’t to be interpreted as saying “most users of PGM wouldn’t be against this change”, which while technically true, is practically not an adequate description of the situation. More accurate would be: “most users of PGM significantly affected by this change, are against it.”

Correct, rotations used to be the only type of pool back in the day, this however changed so long ago, that alternative pool types have existed in PGM for longer than rotations did standalone, with the first iteration of dynamic pools existing shortly after OCC's closure when 1.11 PGM went OS. I don't believe pool commands to have existed back then, but it was the start of changes to the ideas of how the next map should be determined.

With regards to time, this is technically correct. But let’s not forget that for nearly three years, Overcast Network did not exist as a Minecraft server. So while map pools may have existed from early 2017 to late 2019, during this time period awareness of them was diminished compared to when OCC started up in late 2019. So in many users’ minds, the concept of a map pool may be significantly newer than its technical implementation into PGM.

They are proposed to keep their actually legacy functionality: managing rotations, which is what they used to do when rotations were the only way to play maps.

The issue is, this isn’t the legacy functionality that users typically want or associate with the rotation commands. Back on OCN, while it is technically true that the command only worked with rotations, this is because map pools simply did not exist yet. So it was not typically considered that the command only affected rotations, because rotations encompassed all the available types of map pools at the time. And likewise, the newer legacy functionality matches this concept of the commands applying to all the available types of map pools at the time.

Additionally, they're getting fixes for usability that have been rightly criticized.
Confusion on users, not being able to use the command and wondering what happened: This has been fixed by the added alternative usages message, that will point them towards the proper command when rotations aren't available or aren't being used.

Once again, this is the solution to a problem that is invented artificially by this pull request. There is no inherent need to violate users’ expectations of the commands’ performance, and instead push them to this new set of commands. While the clickable command does help with the user experience (several months after it was destroyed), it’s still forcing users to take extra steps to achieve their desired information, even though their intentions are clear.

Servers still using rotations not being able to manage them with /rots etc: This has also been fixed as the commands will work the same to manipulate rotations, just as they did from the very beginning.

This is entirely missing the point. As I explained earlier in more detail, this isn’t the legacy functionality users typically desire, nor does it conceptually match the legacy functionality in question. Additionally, this statement is oblivious to a typical PGM use case: a mixture of different types of map pools in the same config file. If users on such a server want to filter by type of map pool, they are free to utilize the flag-based implementation. But for the vast majority of cases, users are not looking to filter by types of map pools, and would be no better off with the modified functionality. Realistically, demand for this new functionality has been minimal when compared to demand for bringing back the old aliases as they were prior to mid-2022. And if we want to get into semantics, for legacy reasons I believe it’s reasonable for users to consider “shuffled” map pools as another type of rotation. This pull request completely ignores that.

I do not consider the intended behavior of a command called rot or rots shorthand for rotation/rotations, if the user wishes to manage all map pools they're able to use the pool command to get all of them. This is intended behavior, and if the current pool isn't a rotation they'll get a nice message pointing them towards the correct command, which will help users learn the new commands instead of being left without a response or with the wrong expectation. I don't consider it unnecessary to expect the user to use the pool command to manage pools, and as mentioned earlier, they'll get a hint with clickable text to point them towards it.

While this isn’t necessarily intended behavior by those who refactored rotations into map pools, it is certainly the relationship users in recent times have come to expect. And as I explained earlier, this afterthought of a clickable message is a good step in theory, but still forces users to go through extra work to retrieve their information, even though their intentions are already known. If somebody is running /rots, the chances of them desiring all map pools are infinitely higher than them only wanting ordered map pools.

Additionally, doing things like /setrot will tab-in rotation names instead of pool names, which is a new functionality, giving flexibility for when you actually want to set specifically a rotation (eg: funky friday)

So, if by chance funky friday becomes a shuffled rotation, then this feature becomes completely worthless. Unless you implement a /shuffled, which seems like an utter waste of time. In either case these users would be perfectly well-served with the flag implementation. This statement shows that this pull request is genuinely quite out of touch with users’ desires and usage of the commands.

I am trying my best to find a middle-ground that will try to suit all parties involved. Precisely due to the removal not being welcomed it is that i am bringing them back, but trying to do so in a way that will be beneficial to both the plugin and to the users, by informing them about the migration path to the new commands, and making the command fit its naming instead of outright removing it.

This statement comes off as disingenuous when given the context. When the commands were removed in mid-2022, there was definitely opposition to the removal, but it apparently wasn’t strong enough to get these “nice-to-have” transition features added, thus implying that the consequences of the removal were not adequately considered back then. It is telling that we are getting these creative “technically correct” workarounds to implementing what users actually want, only after the users prevailed and got the functionality with the best user experience merged into PGM.

This pull request, when weighing the pros, and cons, is certainly not beneficial to users, and at worst neutral for the plugin. There is minimal user demand for this pull request, as it does not significantly improve user experience, at the cost of just being “technically correct”.

If there's any other problems, bugs or unintended side-effect with the functionality i'm happy to hear them and adapt the code, this is still a draft pull request for that reason. However, feedback that simply back-pedals on "i don't want any change at all, and it must all be kept exactly as-is because i do not want to use pool commands to manage pools" won't be taken into consideration. I hope we can find a middle-ground that will satisfy all parties, for the benefit of PGM, which we all are trying to improve one way or another.

So you are trying to mislead users into thinking that the only option is minute improvements to your idea, and us actually getting the implementation with the best user experience is off the table. This does not imply an invitation for genuine debate.

Since you brought up middle grounds and compromises, let's consider the extremities:

  • No rotation commands.
  • Rotation commands with 2017-2022 functionality.

Now, here is the middle ground you are trying to implement:

  • Rotation commands, but with decimated prior functionality to the point where they're useless in many users' use cases.

Can you see why this may be found unsatisfying? I believe this “middle ground” as it stands satisfies mainly you, but is desirable to very few actual users affected by the change.

@Pablete1234
Copy link
Member Author

As i mentioned earlier, "feedback that simply back-pedals on "i don't want any change at all, and it must all be kept exactly as-is because i do not want to use pool commands to manage pools" won't be taken into consideration."

Unless you implement a /shuffled

Personally i don't oppose the addition of /shuffled and /voted as companion commands to /rots, i hadn't thought of it and it seems like it could fit. However i'm not sure what the singular and plural forms of the commands would be. shuffled and shuffleds? voted and voteds? sounds awful.
If we're going to pursue this one i'm open to suggestions as to how to name them.

@alexsosnovsky
Copy link
Contributor

alexsosnovsky commented Nov 5, 2022

As i mentioned earlier, "feedback that simply back-pedals on "i don't want any change at all, and it must all be kept exactly as-is because i do not want to use pool commands to manage pools" won't be taken into consideration."

Thanks for proving true the final section of my previous comment. But then again, considering that you were beginning to repeat points you previously made, and I had argued against, I'm also fine with the debate ending here. Enough arguments have been made by both sides.

Unless you implement a /shuffled

Personally i don't oppose the addition of /shuffled and /voted as companion commands to /rots, i hadn't thought of it and it seems like it could fit. However i'm not sure what the singular and plural forms of the commands would be. shuffled and shuffleds? voted and voteds? sounds awful. If we're going to pursue this one i'm open to suggestions as to how to name them.

Yet again, a solution in search of a problem.

@lefckntea

This comment was marked as off-topic.

@Pablete1234
Copy link
Member Author

Can you see why this may be found unsatisfying? I believe this “middle ground” as it stands satisfies mainly you, but is desirable to very few actual users affected by the change.

Thanks for proving true the final section of my previous comment.

Yet again, searching confrontation instead of proposing solutions. I'm designing & proposing solutions all you're doing is repeating you don't like change and denying there's a problem at all. So again, if you have actual new design changes or ideas i'm all ears.

Yet again, a solution in search of a problem.

I have simply taken the only proposal you have given, valued it, and developed the conversation around it a bit more in case anyone feels it's worth adding.

@lefckntea
Copy link

doing this impacts and hinders people that use /rots how they're used to and just making rots an alias of pools impacts pablos ego and his
"akshuallly rotation doesnt mean pool"
nerdge argumentation pick your option

@alexsosnovsky
Copy link
Contributor

Yet again, searching confrontation instead of proposing solutions.

This couldn't be further from the truth. I hate and generally avoid confrontations.

I'm designing & proposing solutions all you're doing is repeating you don't like change and denying there's a problem at all. So again, if you have actual new design changes or ideas i'm all ears.

Yes, I am denying that there is an issue with having rotation commands as an alias of pool commands. Finally, you get it! Let me put this situation in simpler terms.

  • The status quo is something people are generally satisfied with.
  • A developer proposes a change, which would negatively affect users.
  • When confronted, the developer is only willing to make improvements to their specific idea, and does not even consider the possibility that their idea isn't necessary in the first place.
    This is where the issue lies. You want people to provide feedback on your idea, but reject perfectly valid feedback consisting of "this is a bad idea, literally just don't do it".

And if we're talking about people searching for confrontation, can we be reminded that we had merged into the dev branch the aliases, and generally nearly everyone was satisfied. There was no need for further confrontation or discussion of the topic. Except you essentially stomped your foot on the ground in protest, being the one person against it, and immediately opened up this PR going against what was agreed upon. And then you have the audacity to accuse me of seeking confrontation.

@Pablete1234
Copy link
Member Author

Yes, I am denying that there is an issue with having rotation commands as an alias of pool commands. Finally, you get it! Let me put this situation in simpler terms.

You're the only one who doesn't seem to get it. I've known all along you don't have a problem with it, that does not mean there isn't one.

The status quo is something people are generally satisfied with.

The pre-january status quo is something you are satisfied with.

A developer proposes a change, which would negatively affect users.

A developer who is dissatisfied with the situation, proposes a change that you view negatively.

When confronted, the developer is only willing to make improvements to their specific idea, and does not even consider the possibility that their idea isn't necessary in the first place.

When confronted, i've been not just making improvements but changing the idea completely and valuing multiple options. The original idea was to simply remove the commands all together. Because valid concerns were raised, i have made solutions to fix the valid concerns, those being, that users may not know how to run the commands if they're just removed, and that servers which still use rotations shouldn't be needing to change. I can agree to those and this solution fixes both.

This is where the issue lies. You want people to provide feedback on your idea, but reject perfectly valid feedback consisting of "this is a bad idea, literally just don't do it".

When the "perfectly valid feedback" consists simply on "i do not want to use the pool command to manage pools" i can do nothing but say sorry, the command to manage pools, is /pools.

And if we're talking about people searching for confrontation, can we be reminded that we had merged into the dev branch the aliases, and generally nearly everyone was satisfied.

When the removal had been merged into the dev branch I was satisfied. When your re-adding was merged you were satisfied.

There was no need for further confrontation or discussion of the topic

Agreed, we could've just kept them gone and have no confrontation, however, you wanted to bring up their re-introduction.

and immediately opened up this PR going against what was agreed upon. And then you have the audacity to accuse me of seeking confrontation.

Agreed upon by who? Also going against what?

I taked to electroid, i told him the situation and he knew how the cloud refactor would kill the aliases. You had also been told the cloud pr would conflict those changes in your pr.
I agreed with electroid to meet a middle-ground solution by making this shortly-after so that they will be re-introduced.

I will re-iterate, yes, you are bringing confrontation here because you dislike the changes, bringing little to no productive feedback.

@alexsosnovsky
Copy link
Contributor

alexsosnovsky commented Nov 6, 2022

I do not consider it necessary to respond to these latest misleading and/or false allegations. My earlier comments should suffice. 👌

@CoWinkKeyDinkInc
Copy link
Contributor

The visual output of both /rots and /pool would be merely identical. I do not see what there is to gain by making a strong distinction in two similar concepts that most end users would see as exactly the same. There is no risk of breaking things because of misunderstanding the difference between a rotation and a (voted) map-pool. Keeping the rotation commands as aliases of their respective pool commands makes sense if the input and output of the commands is the same. They're all prewritten lists of maps that have a different method of selection, the selection of which does not involve the player invoking the commands at all. The only command that does this is /sn, which doesn't involve the pools.

I believe very few pgm users to actually use these commands at all, mostly, map developers. You are one of the people impacted by the change and are accustomed to using the command, as well as talking/interacting with others in the same position, which likely makes you biased towards an importance to keeping them intact that does not hold true for most users.

This is an irrelevant point. Same could be said that you're biased because you don't use these commands, or because you aren't a map developer. That doesn't make a meaningful difference in me agreeing or disagreeing with your proposal.

This issue should have been settled in #1090 when Electroid said that strictly enforcing this technicality outweighs what the end users want, and approved and merged #1090. This pull request is demanding a compromise on your terms, which is bringing back /rot in a different form, rather than having /rot already in the codebase and later proposing to restrict its usage. #1090 was merged before #1080 so there was proper time to fix this merge conflict in good faith. Instead of us discussing if /rot should be readded, now we have to discuss if /rot should be added in the same way as it always did. What was the point of that for?

@Pablete1234
Copy link
Member Author

The visual output of both /rots and /pool would be merely identical.

That's true, i can work on changing it so that the title on the list will say Rotations instead of Map Pools, further defining the difference between the two. I could also include a little X with a hover for "Remove rotation filter" which would run /pools instead, and make it so /pools -t shuffled|voted also have their own custom title indicating they have been filtered, alongside the X to remove the filter.

This is an irrelevant point. Same could be said that you're biased because you don't use these commands, or because you aren't a map developer. That doesn't make a meaningful difference in me agreeing or disagreeing with your proposal.

The only relevant point about it is for when people keep talking about the users instead of speaking for themselves, nothing more besides that. I'm biased but not because i don't use the commands (i do, and use the correct ones), but rather because the commands are simply incorrect.

Electroid said that strictly enforcing this technicality outweighs what the end users want

Yes, and he did so without taking into consideration alternative approaches like this one, which, when inquired about, he was ok to accept.

#1090 was merged before #1080 so there was proper time to fix this merge conflict in good faith.

Electroid was aware of the command not being there, and i had already informed in #1090, and this is a 2nd round of fixes for cloud commands which brings them back.

Instead of us discussing if /rot should be readded, now we have to discuss if /rot should be added in the same way as it always did. What was the point of that for?

Rot had been removed months ago, and the discussion is now about how it gets re-added so that it won't be a problem long term. Map pools and rots are different concepts, one is a generalization, one is a specific term, and the commands have to reflect that to not mislead users into forever keep calling all types of pools rots. I think that being an issue has been shown by some of the people opposing these changes, with some of them unable to see the problem at all or saying pools and rots are the same.

@Pablete1234 Pablete1234 force-pushed the reintroduce-rot-cmd branch 2 times, most recently from bf05ecf to 78136e9 Compare November 19, 2022 03:26
Signed-off-by: Pablete1234 <[email protected]>
@Pablete1234
Copy link
Member Author

Superceeded by #1100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working reintroduce Reintroduces a feature that was removed
Development

Successfully merging this pull request may close these issues.

5 participants