-
Notifications
You must be signed in to change notification settings - Fork 68
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
new loot dialog options: rare items #23
base: master
Are you sure you want to change the base?
Conversation
The idea is to allow showing dialog/notification for rare/quest items, while still not showing any notifications otherwise. Notification logic has been re-factored a bit.
UPD: offtopic deleted. |
It would be better to post the bug reports on the Andor's Trail forums. Stoutford is in development, and at this point nothing should be taken as much more than a placeholder. The gloves and shoes are not as powerful as you think: note the actor condition. The stats you see now are also not necessarily final (in fact, it's quite likely they will change). There is a quest in Stoutford, as well as three other new ones in the stoutford_tests branch that can be completed. |
To reviewers. Note that the old code had some strange parts I'm not sure I reflected 1-to-1 in the new code. For example, there was intersection between "show notification" and "show dialog" events. And one did not fully include the other. The logic was also split across files. |
@@ -6,7 +6,7 @@ | |||
|
|||
public final class ItemType { | |||
|
|||
public static enum DisplayType { |
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.
Why remove the "static" modifier ?
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.
There's just no need for that, since inner enums are static anyway. http://stackoverflow.com/questions/663834/in-java-are-enum-types-inside-a-class-static
But I guess if that's a common code style in AT, I could revert that part.
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.
@Zukero should I revert that part?
@Rijackson thank you for your answer. You may be right, I didn't test Stoudford throughly. You probably have to have some other quests completed at the time you enter Stoudford, or maybe I didn't click/notice something (yet). |
There are no specific requirements to start the new quest in Stoutford. Which boss monster do you mean in Flagstone? The Winged Demon in the last map? |
@Rijackson I'll have to re-check. It seemed like a village where inside each house there is either nobody inside, or just a friendly man who's not talking much. Apart from the priest who is just angry. Again, I'll re-check and try to talk to everyone yet again. About Flagstone - yes, that's the one (the second, last demon). After the demon there is only the villain inside (to whom you can talk). Also note I have the "optimized drawing" checked in the settings. |
Could you open a bug on the forums with details including a screenshot for
that demon please ? I'd like to know more.
Le 19 janv. 2017 8:36 PM, "Vasya Novikov" <[email protected]> a
écrit :
… @Rijackson <https://github.com/Rijackson> I'll have to re-check. It seemd
like a village where either there's no one inside any houses, or he's just
friendly and not talking much. Apart from the priest who is angry, but
that's about it. Again, I'll re-check and try to talk to everyone.
About Flagstone - yes, that's the one (the second, last demon). After the
demon there is only the villain inside (to whom you can talk).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#23 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFG_z6ZUeNL4sf55dADVxnkICLE6a3ncks5rT7ssgaJpZM4Lj5yo>
.
|
No need to revert. Your justification was good. I learned something. Thanks.
Le 19 janv. 2017 8:34 PM, "Vasya Novikov" <[email protected]> a
écrit :
… ***@***.**** commented on this pull request.
------------------------------
In AndorsTrail/src/com/gpl/rpg/AndorsTrail/model/item/ItemType.java
<#23>:
> @@ -6,7 +6,7 @@
public final class ItemType {
- public static enum DisplayType {
@Zukero <https://github.com/Zukero> should I revert that part?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#23>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFG_z8Jl-3v938DZoAjk7f4GhuT4bmZbks5rT7qMgaJpZM4Lj5yo>
.
|
EDIT: deleted offtopic question. |
Yup. I found the cause and I think it is solved. Need some more testing,
but it's pushed to github already (both master and stoutford_tests).
Le 21 janv. 2017 12:10 PM, "Vasya Novikov" <[email protected]> a
écrit :
… @Zukero <https://github.com/Zukero> @Rijackson
<https://github.com/Rijackson> Screenshots of the counter-intuitive (for
me) Flagstone demon drawing:
[image: screenshot_20170121-140133]
<https://cloud.githubusercontent.com/assets/1547617/22173985/ac1cd3f0-dfeb-11e6-9950-e896a45005ae.png>
[image: screenshot_20170121-140154]
<https://cloud.githubusercontent.com/assets/1547617/22173986/ac46dea2-dfeb-11e6-8b5d-fa905ce089f8.png>
[image: screenshot_20170121-140247]
<https://cloud.githubusercontent.com/assets/1547617/22173987/ac47ed88-dfeb-11e6-9346-ca3199f9c6ef.png>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#23 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFG_z01Y2pl9VNKZXh2Uu_0VAlOEBrFDks5rUee-gaJpZM4Lj5yo>
.
|
I see that Andor's Trail is still alive, yet this PR is kinda stale. |
Because it was opened concurrently to a huge UI overhaul and that it is currently unmergeable :) |
Upd: it seems that I no logger have the android SDK, and setting it is a non-zero task for me. I'm afraid I'll have to step down from this task..
…On April 6, 2018 8:22:06 PM UTC, Zukero ***@***.***> wrote:
Because it was opened concurrently to a huge UI overhaul and that it is
currently unmergeable :)
That UI overhaul was recently merged, so if you can find the time to
update your PR, I'll be happy to merge it.
--
Sent from my mobile device. Please excuse my brevity.
|
No problem. Thanks for submitting this in the first place. I'll try to find time to fix it myself. |
PR to get actual
The idea is to allow showing dialog/notification for rare/quest items,
while still not showing any notifications otherwise.
Notification logic has been re-factored a bit.