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

Fixed for https://github.com/beyond-aion/aion-server/issues/32 #65

Open
wants to merge 2 commits into
base: 4.8
Choose a base branch
from

Conversation

sky1683589933
Copy link
Contributor

Fixed for #32

@Yoress
Copy link

Yoress commented Feb 26, 2025

Let me preface this with a brief disclaimer: I've not looked at the related code; nor, have I tested the changes made for this PR. So, things I bring up in this comment may or may not be relevant.

#32 happens in a context that's not tied to any quest; so, why would fixing the problem involve changing quest data?

It seems like the changes requested here simply alter which quest handler is in charge of quest 2107? That would only prevent #32 from happening again on the particular NPC listed; but, would actually bury the underlying problem. Why is the item_order quest handler attempting to handle dialogue requests that are not related to a quest at all?

As for the data change, if the original data is actually incorrect, then that's a separate problem that would conventionally be dealt with in its own issue and PR. At least, under the assumption that the line of thinking I presented above is correct (and, as disclaimed, it may not be!). Same thing with the quest start item for quest 2107 appearing to not be listed in the drop rules for Ishalgen.

@sky1683589933
Copy link
Contributor Author

sky1683589933 commented Feb 26, 2025

Let me preface this with a brief disclaimer: I've not looked at the related code; nor, have I tested the changes made for this PR. So, things I bring up in this comment may or may not be relevant.
#32 happens in a context that's not tied to any quest; so, why would fixing the problem involve changing quest data?
It seems like the changes requested here simply alter which quest handler is in charge of quest 2107? That would only prevent #32 from happening again on the particular NPC listed; but, would actually bury the underlying problem. Why is the item_order quest handler attempting to handle dialogue requests that are not related to a quest at all?
As for the data change, if the original data is actually incorrect, then that's a separate problem that would conventionally be dealt with in its own issue and PR. At least, under the assumption that the line of thinking I presented above is correct (and, as disclaimed, it may not be!). Same thing with the quest start item for quest 2107 appearing to not be listed in the drop rules for Ishalgen.

hey, It is necessary to mention the issue in your report, because if I do not mention it, once the PR is merged, the issue in your report will not exist and no one knows how it disappeared.
As for whether PR is correct, it requires very rigorous review. @ Neon is a very professional developer, and I have a deep understanding of this, so there is no need to worry!
PS: My current spare time mainly involves fixing some quests and related bugs. Coincidentally, the issue you reported may seem to be related to the quest conversation to some extent, or it may be a problem with the public conversation. However, I will not spend time investigating the public conversation because in my opinion, this is not a key issue, but just icing on the cake!

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

2 participants