search

RuneMate / Issues

Client and API Issue tracking. See README for more information.
8 stars 0 forks source link

Log the reason that the bot stopped #53

Closed simwij closed 5 years ago

simwij commented 5 years ago

Describe the bug The bot.stop(reason) method does not relay the reason in the logs or the UI. What is the point of a reason message if it is not used?

Affected game modes: Both

RuneMate version: 2.96.4

To Reproduce

  1. Use the bot stop method with a reason message
  2. The bot stops without relaying the reason.

Please include the code used to produce this bug.

// Kotlin
Environment.getBot().stop("The bot stopped because of yadayada")

Expected behaviour The logs should contain the stop. They don't. Perhaps it would also be good to show the reason in the UI.

EJ-K commented 5 years ago

This is not a bug.

This was a feature introduced to display back when RuneMate WebServices are a thing. I feel it would be a mistake to display these as ClientUI notifications whenever stop() is invoked, as ClientUI notifications should only be displayed when explicitly requested by the author.

If we haven't already, we could include the reason into PushBullet notification, but that should be raised as a separate issue (feature request).

EJ-K commented 5 years ago

Re-opening for discussion.

@ccarpenter04 Perhaps we could remove the deprecation flag from the original stop method so that a reason is no longer required?

ccarpenter04 commented 5 years ago

It is used in combination with pushbullet to notify users that a bot has been terminated and is desirable to have for future services. That being said, I could tolerate a default shutdown message as long as it wasn't used for error state oriented shutdown

ccarpenter04 commented 5 years ago

The problem presented in this issue though is that the reason isn't being logged. Regardless of other uses everything should be logged somewhere.

simwij commented 5 years ago

This is not a bug.

This was a feature introduced to display back when RuneMate WebServices are a thing. I feel it would be a mistake to display these as ClientUI notifications whenever stop() is invoked, as ClientUI notifications should only be displayed when explicitly requested by the author.

If we haven't already, we could include the reason into PushBullet notification, but that should be raised as a separate issue (feature request).

Yes, you are right about the feauture request. The issue is about it not showing anywhere, in particular the logs.

ccarpenter04 commented 5 years ago

Implemented