-
Notifications
You must be signed in to change notification settings - Fork 558
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
flag (no interact) quest GIVER Game Object when player not eligible for that quest #157
Conversation
*data << uint16(0); | ||
} | ||
else | ||
*data << uint32(0); // disable quest object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very small thing but should move this comment to line 520 as it applies to the whole else statement.
@@ -517,8 +517,18 @@ void Object::BuildValuesUpdate(uint8 updatetype, ByteBuffer* data, UpdateMask* u | |||
break; | |||
} | |||
} | |||
else | |||
*data << uint32(0); // disable quest object | |||
else // flag (no interact) quest object when player not eligible for quest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as requested, move comment up to else as it applies to the whole else statement.
Although this fix appears to take care of most of the quest giver objects, such as wanted posters and the Conspicuous Gravestone in Gadgetzan etc. It should be noted that quests like the 'Old Lion Statue' (http://db.vanillagaming.org/?object=31), where the player interacts with this quest giver GO, and a progress bar (like opening a chest) is activated and then once the progress is up the quest dialog/text is displayed, even after this change the default greeting message is still displayed. Perhaps this is correct behavior and that quest just needs more appropriate quest text when the player is not eligible for the quest yet? However if this is not the case, the interaction should be flagged here as well. It might require doing somewhere else as it does not appear to be handled at Object.cpp, Object::BuildValuesUpdate(...) where the other quest giver game objects are now flagged. For the 'Old Lion Statue' , the quest game object the logic that handles showing the quest text is rather in SpellEffects.cpp, Spell::EffectOpenLock(...)-> Spell::SendLoot(...)., I am not sure how we can flag it appropriate or if it is even possible here? or as said earlier, it could be correct behavior now and more appropriate quest text is required? |
I think it should have "untargetable" flag in DB, which would make it targetable only when on the quest? |
Not sure if we make quest GIVER game objects 'untargetable'? Is no interact and untargetable going to have the same effect? quest game objects/items Vs. quest giver game objects appear to be managed subtly different from what little I was able to determine when looking into this pull request but it is most likely I missed the big picture in how it all work together. For example, if the quest giver object was untargetable instead of no-interact, would that mean the player can still mouseover the wanted posted and see information on it etc. ? If it solves the problem and works in better with the existing way quest giver objects work I am all for it =) I just have no idea how to go about it. I do mind rolling my sleeves up and trying it out but I might need a few hints on where to dig deeper etc. |
Yes, we definitely make quest GOs untargetable, I was fiddling around with it yesterday. But that is not to say there cant be an exception. (example is chest in lakeshire in the lake is untargetable when not on quest) |
Was that the quest for Oslow's Toolbox on the The Lost Tools quest where your find the Sunken Chest ? if so, although, I maybe being overly pedantic on terminology, these are different, and from what I can gathered, handled differently, in that the the sunken locker is a quest game object/item, where as say a wanted poster is a quest GIVER game object. i.e GAMEOBJECT_TYPE_QUESTGIVER Vs. GAMEOBJECT_TYPE_CHEST (which does not always have to be a literal chest e.g. bingles toolbucket etc.). This may not matter, in that we treat them the same regardless? I cannot recall how vanilla retail treated say a wanted poster which is expected to prompt the player with quest message when appropriate versus a quest game object that completes a quest or makes quest loot available when clicked, if appropriate. |
Hmmm, I see where you are coming from. Well a wanted poster should only be empty, but if it were a statue, I would say it shouldnt be "targetable" if it doesnt have gossip_menu text |
OK. I will fiddle around to see if I can make cases like the old lion statue behave more like a traditional quest game object and become untargetable when there is no gossip menu text. As you suggest that would more likely be a database flag change. Currently I still believe this particular pull request stands for quest GIVER game objects unless i can figure out a way for them to also all be handled appropriately with db flagging. |
Sure, I just suggested what to investigate :) |
Assigned myself. I'll try to review this next week. Thanks. |
Is there any prerequisite DB-side before testing this PR? In its current state, players can still interact with quest GOs even not on the related quest(s) (or I missed something while/before testing). |
There is no DB-side for the pull request. In an nutshell, as it stands now, all it does is suppress the 'Greetings playername quest dialog box if the player clicks on a quest GIVER GO (e.g. Wanted Poster's/tombstone) and they are not applicable for that particular quest. As stated above, this is different to a quest GO, a quest GO does not allow any interaction or mouse-over feedback unless the player is on the relevant quest and/or quest chain. For example, a wanted poster, which is quest GIVER GO, will still give some feedback (e.g. 'Wanted blah...' if you mouse over it, even if you are not applicable for the quest but will not present you with the quest dialog until you are applicable. Having said all that, it should noted that this may not be the desired behavior and that we want to treat quest GIVER objects the same as quest GO's, that is no interaction/mouseover feeback etc. at all if the user is not applicable for that quest pickup. I believe that is how it is currently handled on Kronos 2 but my memory of how it really was on retail classic is hazy at best. I do know of at least one Quest giver Go that will still present you with a greetings dialog after applying this pull request and that is the 'old lion statue' in Redridge. see previous explanations of why that maybe so... If during your testing after applying this pull request you are still seeing quest giver objects showing the default greetings dialog, let me know which ones they are and I will take a look at it. |
Thanks for clarifying. That's what I understood at first and unless I missed something, the tombstone near Gadgetzan and the Wanted poster in Thelsamar are still displaying the "Greetings $n" default text. Hence, my question. I'll try to build it anew. |
*Edit: quoted the wrong part of your message there, sorry. I think this is WRONG. My motivation for this trail of thought is that such a change would make players unable to hover the mouse over quest giver objects where the player is not high enough level to start the quest, yet the player will still see the silver exclamation mark. Those 2 things won't ever go well together if you ask me. |
I was probably not clear and unfortunately the terminology does get overly pedantic with quest GO's versus quest giver GO's but I think we are on the same page as this pull request should allow the user to mouse-over quest giver game objects at all times (levels, not on quest etc.) to get feedback on what the quest is, if it is working properly. The only thing it should suppress, is the quest default 'Greeting's palyername' if the player is not applicable. |
I know this Pr does not include the change you mentioned which I quoted, but you very much pondered the change, and so I thought I would voice my concerns with that; and only that. This PR itself 👍 |
I am not attached to this way of doing it, if someone else has a clear recollection of how the player interacted with the various different kinds of game objects (quest, quest giver, chest opening etc. ) and if they were treated all the same or how the deviated in their interaction, let us know and I will be glad to try and implement it that way =). If nothing else, this hopefully gets us thinking how the system should more closely react like classic retail in these circumstances, besides the current behavior of a presenting a quest dialog with a default greeting's playername message. I'm all for whatever works best. |
@cala how did it go after a new build, were you able to see the desired result or are you still seeing the old behavior? |
Still having the old behaviour with GO 148504 (A Conspicuous Gravestone) and 185166 (Wanted Poster). I tested a few more and it was the same. Weird. :/ |
That is strange. I am pretty sure my cmangos-classic source code wise, was just the latest build when the pr was made with just the pr changes applied, however, I cannot speak for the how pristine my database was at the time, although it should not have deviated too much from a stock standard one. Having said that, even if there were 'custom' changes in my db, it should not have influenced this pr unless there was quest text message for the quest but if that was the case you would not get the default greeting message anyway. I will get a clean build of everything myself to be sure, test it without and then with pr and see if there is something else going on. |
Argh!! Like you I am now witnessing the old behavior of the default greeting message quest dialog even after I apply this PR to the latest and greatest (code and database) but if i go back to an older copy (when I made the PR) it works as desired. Unfortunately the older copy I am testing with is just a binary backup with the PR applied. For now it is best to stop testing on this and put it on hold until I figure out what has changed and if it is still worth pursuing. I guess i could start git bisecting from that earlier build to now and see what may have changed that could possibly influence this, it is definitely nothing to do with the immediate code changes as Object.cpp has not changed in months. It does beg the question that if it it some 'unrelated' change that can influence this behavior with the PR applied, this PR change might have been a too brittle a modification to make in the first place with the potential to break again in the future regardless of what I find. I will keep you updated if I discover anything. Sorry for the inconvenience. Its one of those 'but it works on my machine...' ;) |
No worries! GO testing is always a bit tricky because it seems that their cache is not always cleaned up as it should. I wanted to be sure that that was not the case for me. I let you look if you can find what has changed and what was previously working. Thank you for your PR and all the explanations/testing that went with it anyway. 😄 |
Well the plot thickens but for now I will just say that the primary problem with this pull request implementation was between my chair and keyboard. In a nutshell i messed up snipping between multiple repositories I maintain and was not being over diligent and testing across all of them. My apologies. I have another idea on how to achieve the same desired results. I am pressed for time at the moment but will endeavor to submit another pull request on the weekend, I will leave this one open for a reminder until I submit the new one. |
😄 You don't have to open a new PR, you can fix this one simply using git; #119 (comment) |
Looking forward to this still, hoping you havn't given up on it. |
Not given up on but had definitely relegated to the back burner... I will take a look again on the weekend and submit what I had working, although it did not utilize the previously suggested GO_DYNFLAG_LO_NO_INTERACT, I was never able to get that to work. Maybe someone else will chip in if I submit a PR not using it and can show how to use GO_DYNFLAG_LO_NO_INTERACT correctly. Thanks for the reminder. =) |
Work is consuming my time, blah blah blah... Long story short, I will try again this weekend, apologies for the delay. |
No hurry, no worries. Take the time you need to work on this with a fresh and clean mind rather than doing it in a rush or in unmotivated state of mind. Remember: we are doing this for fun. 😉 Thank you for the update anyway. |
After I finally created a Trinity PR for this some new information surfaced: TrinityCore/TrinityCore#17791 you should definitely take a look! |
@cala i think we can close this as it seem a simple db could fix that. TrinityCore/TrinityCore#17803 |
@cyberium I test the SQL query in the link you provided and it did not fix the issue. |
Ok so it may require some core part. Could you confirm that the request suggested is correct? I mean can i use the flag suggested by that request in the core? Edit: just tryed this on tbc and classic and it worked for the gadjetzan graveyard -- on tbc
mysql> UPDATE gameobject_template SET flags = flags | 4 WHERE type = 2 AND Data3 = 0;
Query OK, 314 rows affected
Rows matched: 361 Changed: 314 Warnings: 0
-- on classic
mysql> UPDATE gameobject_template SET flags = flags | 4 WHERE type = 2 AND Data3 = 0;
Query OK, 259 rows affected
Rows matched: 263 Changed: 259 Warnings: 0 Ah, i may need to check now if it correctly actived if the condition is fullfiled. |
hi Also... i looked on old sniffs: i have one from 3.3.0
I also checked that grave and poster in tanaris... |
If it does not happen in wotlk I would seriously suggest checking code handling classic vs wotlk. |
Cyberium just made me test it and I cannot report any issues with it, was working fine. |
So, if nobody still get this issue, we can safely check it off our list, then. |
Not occuring on my end after setting flag. |
Using GAMEOBJECT_DYN_FLAGS will flag quest giver Game Object (GO) GO_DYNFLAG_LO_NO_INTERACT if the player is not eligible for the quest. Lack of eligibility is determined by the player not being active to quest and there being no quest text to display.
This was motivated by the need to suppress the default greeting text on various quest giver game object such as wanted posters when the player clicked on these items but was not eligible for that quest or that part of a quest chain etc.
See WANTED Poster Greets You #142 and Gadgetzan #860 for more history and details motivating this pull request.