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

Changed how share progress work #1570

Closed
wants to merge 3 commits into from
Closed

Changed how share progress work #1570

wants to merge 3 commits into from

Conversation

AlessioDP
Copy link
Collaborator

No description provided.

@PikaMug
Copy link
Owner

PikaMug commented Jan 15, 2021

Thanks for your time on this. I'm confused about a few aspects of the request:

  • I see share levels have been removed. Was there no alternative?
  • "Fellows" is not a common word in English. Would you consider "Friends" or "Members"? Done, thanks!
  • Why remove the ability to set Use Parties / Use DungeonsXL per quest?
  • "Pick level of progress sharing" and such should not be hardcoded, please add to strings.yml instead.

@AlessioDP
Copy link
Collaborator Author

Pushed some changes:

  • Changed fellows into friends
  • Some javadoc changes

About questions:
The share level system is complicated to handle and I opted for one type of sharing, the global one. Objective/Stages/Quests require some direct changes to the Quester object and can be buggy (Quest one should be the easier).

Parties and DungeonsXL friends are calculated before quests iteration but its possible to add some options to filter them at quest-level, so you can decide which friend to use per quest.

@PikaMug
Copy link
Owner

PikaMug commented Jan 16, 2021

Continuing our Discord discussion, I found time to experiment using multiple game instances. It actually seems your original methods (in master) are working just fine. All share levels function correctly (tested Break Block and Kill Mob), and I was able to get the Catch Fish objective that you and DeadlyKill had problems with working, see 87d847f

Am I missing something?

@AlessioDP
Copy link
Collaborator Author

You can see clearly that there is a problem with this setup:

  • 2 quests with the same objective (fish 10 and fish 100 for example).
  • Player A with only one quest and Player B with both

When you try to fish can happen a lot of things based on how quests options are setup: double incrementation, share only same quest not working

And most important, if Player A doesn't have a quest or doesn't trigger it, the quest of friends doesn't increase. (Player A no quest, Player B fish quest, Player A fishes, no increase)

@PikaMug
Copy link
Owner

PikaMug commented Jan 16, 2021

Thanks for the clarification. Just went fishing and had no trouble with either situation. Player A and Player B both had different quests each, both with a Catch Fish objective, and they all incremented after catching one fish. I also tried Player A with only one quest and Player B with both, worked fine. Didn't experience any double incrementation.

Even Player A having no quest also worked like a breeze - Player B received progress for the catch. I'm just not seeing any issues.

My testing setup is as follows.

  • This server is running Paper version git-Paper-414 (MC: 1.16.4)
  • Quests https://ci.codemc.io/job/PikaMug/job/Quests/199/
  • Parties 3.0.0-rc.2 (default everything)
  • Citizens 2.0.27
  • EssentialsX 2.17.0.0
  • FAWE 1.16-455
  • Vault 1.7.1-b91
  • WorldGuard 7.0.2

Quests file: https://pastebin.com/nbBDKysC
Server is offline-mode with two clients connected using MultiMC. Both players are in the same Party.

@AlessioDP
Copy link
Collaborator Author

I am not able to replicate it with your files: when I use the fish rod, automatically increase quests of the members to 2/2 and 2/3.

So I changed my quests.yml into a block break event, easier to handle. And this happened:

About same-quest-only issue:
On the left, 1 quest only and break a block. On the right, 2 quests.
Screen

About double incrementation one, its replicable with the same config but with the opposite player:
On the left, 1 quest only. On the right, 2 quests and break a block:

Screen2

quests.yml: https://pastebin.com/aQpdASp9

PikaMug added a commit that referenced this pull request Jan 17, 2021
@PikaMug
Copy link
Owner

PikaMug commented Jan 17, 2021

For the same-quest-only issue, that's the intended behavior? The "left" quest has same-quest-only: true so it only increments partyTest for both players. fishTest is intentionally not incremented.

I was able to replicate the double incrementation there. Please see my commit 49da2b0 for a tested fix of Break Block only.

Build: https://ci.codemc.io/job/PikaMug/job/Quests/200/

@AlessioDP
Copy link
Collaborator Author

No, its not the correct behaviour. Same quest true means that the quest must be incremented only if the other player have the same quest.

partyTest increased correctly. fishTest didn't (and it have the same quest option on false).

About the double incrementation, the issue is fixed but its a work around :P
But I don't think it will work with a setup like this (didn't test):

  • Quest A (only same quest on)
  • Quest B (only same quest off)
  • Quest C (only same quest on)

The multiplayer dispatch will be triggered only on the first one (your fix), and will increase correctly A and B:
A because its the same quest; B because have only same quest off.
The C quest will never be triggered for friends because dispatched boolean will be on true.

@PikaMug
Copy link
Owner

PikaMug commented Jan 17, 2021

Thanks for your patience, but I thought the other player does have the same quest? At least, it looks that way from the screenshot:
edit

Unless you mean to say that the second player should have only the same quest, and no others. In which case, we have different interpretations of how that setting should work. Imo forcing the player to exclusively hold a certain quest should be enforced with quest blocks, and maybe the max-quests config setting.

I'll go ahead and test that ABC scenario, but I think you're probably right. Will update soon.

Edit: You were correct. See latest commit

@AlessioDP
Copy link
Collaborator Author

The meaning of same-quest-only should be: This quest can be shared only with who have the same quest.
It must not impact with other quests.

@PikaMug
Copy link
Owner

PikaMug commented Jan 17, 2021

OK, that's what I thought. It appears to be working correctly, then.

@AlessioDP
Copy link
Collaborator Author

Tested a little bit, with this quests.yml its not working properly: https://pastebin.com/N0dJapNe

Quests: Test1 (share true), Test2 (share false), Test3 (share true)
Players: PlayerA (Test1, Test3), PlayerB (Test1, Test2, Test3)
Block destroy: PlayerA

Its not increasing the quest Test3

@PikaMug
Copy link
Owner

PikaMug commented Jan 18, 2021

I'm not able to replicate that issue using your sample. PlayerA breaking an IRON_ORE block worked fine:

break

Only the test2 quest did not increment, which is correct in this scenario (because PlayerA does not have that quest).

Edit: For your convenience, https://ci.codemc.io/job/PikaMug/job/Quests/201/

@AlessioDP
Copy link
Collaborator Author

Test2 should increase by one because the option same-quest-only is on false.

So any iron break event should increase that quest.

The meaning of same-quest-only should be: This quest can be shared only with who have the same quest.
It must not impact with other quests.

Right now its behaviour is like same-quest-only: true.
Test1 and Test3 are increasing correctly because who destroyed the block, have the quest. But the other one should increase even if the destroyer doesn't have the quest active.

About my previous test, probably I confused Test3 with Test2.

@PikaMug
Copy link
Owner

PikaMug commented Jan 18, 2021

Sorry for the misunderstanding. I see what you meant and have made another attempt. See dcb0d8f

https://ci.codemc.io/job/PikaMug/job/Quests/202/

@AlessioDP
Copy link
Collaborator Author

Perfect now it works if PlayerA always have at least one quest about block break :)

But in the case PlayerA doesn't have any quest (or for example he already completed one) after a block destroy, PlayerB quests don't increase.
Warning: Only same-quest-only must be on false to work like this.

Screen

@PikaMug
Copy link
Owner

PikaMug commented Jan 18, 2021

I would say that's acceptable. Even as a Party member, they ought to be participating in quests somehow. In other words, we can interpret same-quest-only: false to mean they must still have at least one quest. In fact, this might discourage a server's older players from inviting new ones to their party only to use them as free labor before they understand how Quests works.

@AlessioDP
Copy link
Collaborator Author

I don't really know, that would be a good feature. To avoid the "free labor" thing you can setup a distance, so the quest is shared only if you are near the main quester.

Another great way to use the same-quest-only false is when you have a big quest like: Break 1000 stone.

Player A is 800/100 and Player B is 900/1000.
Player B will finish first, then he can continue to mine to help Player A to finish it as well.

I mean, personally I would prefer the same-quest-only false thing a must, because not every member is always doing the same quest.

@DeadlykillOfficial
Copy link

As pure example on my Server docs is. I have a Custom item that has 0.1% drop chance on my server and it gives special quest to the player that is to kill world boss. The way my server handles World boss is. It requires 20-30 people Raid in order to be able to have any chance versus this Mob. Leaving it 1:30 ratio chance for this player to be able to Last hit the Boss and get the quest done. In such case scenario when not having same-quest: false option. This would be nearly impossible to make as nobody can keep track on boss that has 3-4 Million hearts and it takes decent 30-40 minutes to kill.

Other example of that would be like Alessio mentioned. Large quests like Mine 1000 Stone or even Mine 50,000 Stone. It can significantly help so people can help each other even when one of the players has finished the quest already or another person it's not eligible for it.

We spoke with Alessio about the Distance feature. Which can pair with same-quest: false. Leaving it like Only party members within 50 blocks of Quester to benefit progress to his quest. Which means. If you ever have "labor" in your server you would need to have them near you in order to "work for you"

In general same-quest: false option would benefit a lot those "rare tier" quests that are obtainable only through non-traditional quest obtain methods...

@DeadlykillOfficial
Copy link

As for Options 2-3-4 from current quest. They were originally added but made no sense at all to be used ever.
Everything is basically how every RPG player would understand how quest is handled. Games like WoW also do that same way. Even tho you can play quests with friends. You cannot finish entire Stages/Objectives or even entire quest for another player. Sure you might be able to help them, whenever you have had already finished the quest. But definitely not finish it for them upon your personal completion.

The way new Parties branch and code from Alessio seemed to do perfect job while we was performing the tests.
It was handling also further issues like rod right click spam increasing count, catch finish double increasing fish count for party members. Killing player giving double count to party members and no to leader and etc...

I'm personally thinking the way Alessio branched the stuff is more efficient and better to use. And the option Everything in my opinion should be basically the only one available.

@PikaMug
Copy link
Owner

PikaMug commented Jan 18, 2021

Thanks for the input @DeadlykillOfficial and yes, many popular games use the Everything style of progress distribution. However, a time-sensitive quest might wish to hurry players along on the same Objective as it progresses, same of boss fights with multiple forms or Stages. Other servers could prefer to let older players carry others through difficult areas, so once certain Quests are completed, even the newer players are rewarded.

One might say those are niches and the Everything setting will cover most (which is why it's default), but I am confident there will be those who wish to do things differently. I'm fine with reducing customization if necessary, but I felt there might be an alternative and currently my idea works and also addresses the count issues. As far as efficiency I'm fairly certain we can make distribution asynchronous later on.

The (seemingly) final issue is the example from @AlessioDP

Player A is 800/100 and Player B is 900/1000.
Player B will finish first, then he can continue to mine to help Player A to finish it as well.

If Player B no longer has a quest and same-quest-only: false then he can't help Player A. In this situation, I feel an Objective distribution would be the solution here. Consider that some servers might not want Player B to be able to help Player A in this case - if we fixed this "issue" then that server would have no recourse. We would possibly then be asked to add some kind of same-party-override option later on. In other words, should being a member of the same party dictate whether to help Player A, or should having a similar quest dictate? I personally feel like it's the latter.

@DeadlykillOfficial
Copy link

Objective Kinda makes quests meaningless in such case. Cause giving you pure example of World of Warcraft. You have T6 quest that requires you to kill 5 bosses. Correct? During the raid people leave and there must be replacements right? This means this new replacements either have killed 0/5 or 1/5 bosses while initial raid is already 4/5 which means there is 1 boss left for initial Raid and 4 left for extra people that joined later. Having Objective in such case would make the entire quest meaningless because since Initial raid finish the quest, it would auto-finish it for other people making them literally "jump" or more like "exploit" through that quest.

We've spoke with Alessio and got into conclusion that initially the current parties branch does all the job without any issues and it handles stuff differently and Alessio can add different options later on including the Distance option and many more.

From my many years experience of MMORPG games. And my entire Quests experience and my time since Parties were introduced to quests. I've honestly never heard anyone ever having usage of Option 2-4 for Sharing. Everyone mainly uses "Everything" as this is the most understandable Quest sharing option that 99% of RPG games have and it's accepted as normal sharing way.

As for Boss example. The Quest was entirely Exclusive rare quest. That's available to 1% of players in the server. Which means from 30 people raid. There will be most likely only 1 person that will have the same quest. Therefor same-quest: false is required.
Because you can't guarantee that this specific person will last hit the Boss. Especially if the boss has 5 million hearts and it's taking decent 30-40 minutes to kill as 30 people raid.

@DeadlykillOfficial
Copy link

Also same-quest: false can help a community quests. Which are basically Master tier Quests including custom plugins.
Which would be for example: Mine 50000 stone per player as reward it execute global command that fills values of other plugin upon reaching like 500 value it does execute or does anything in the server. Like lots of custom stuff. And trust me mining 50k stone solo would be incredibly hard. Also having Labors help for the common good of the server it's why same-quest: false it's a good option to have. This means people that are incredibly dedicated to finish the Server Goal and receive like Legendary Item. He can simply join other people parties and voluntarily help them finish the quest so it speed up the process of obtaining this item.

@DeadlykillOfficial
Copy link

Could you briefly explain what's stopping you from pulling Alessios rewritten system of how entire MP works and it's fully tested every event everything. And then letting him basically add the features later on. As Stages/Objectives and etc. We are having Discussion and trying to find solutions of already fixed thing and already coded. Yes, at the moment it's missing several Quests default options but they will be readded shortly. Could you explain why you are trying to fix and keep the old code of something that has not been working for so long and even right now, it's not working as originally intended. While the new code does exactly what it's supposed to do and it's coded in a way for easy expanding on it's features.

@AlessioDP
Copy link
Collaborator Author

I see that the old system can be tweaked and fixed so it works but is it really necessary?

I can implement the share level thing in the new one too xD
I just have to check for the Level == 1 and re-add previous deleted methods to handle different levels.

About the "no-quest player that cannot help others": its the same thing of "same-quest-only":

  • Player A have a fish quest
  • Player B have a stone break quest (same-only-quest on false)

If Player A breaks a stone, it will help Player B to progress. This thing works.
But if Player A doesn't have quests at all, he cannot help Player B to progress, even if the quest of Player B have same-only-quest on false.

@PikaMug
Copy link
Owner

PikaMug commented Jan 19, 2021

@DeadlykillOfficial I expressed this in my first reply: removing features seems unnecessary if a reasonable alternative exists. I was totally prepared to accept this PR if it came to that, but I've found a solution that lets users keep the current level of customization. Your examples make sense for a standard WoW-like quest, but the ultimate goal of this plugin is to let users design quests however they like. If an operator wants players to independently mine 50000 stone, but award everyone at once when somebody hits that goal (like a contest), removing the other options makes that implausible. Imho the few ms performance benefit of this PR is nice but not substantial enough to outweigh perceived benefit.

@AlessioDP If you're able to implement the old features as well, by all means. I appreciate both of your input and the effort you've put in here. Should my solution have some major unforseen issue, we can certainly come back to this PR.

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.

None yet

3 participants