Skip to content
This repository has been archived by the owner on Feb 2, 2022. It is now read-only.

AFK players handling and diverse modifications #17

Open
wants to merge 57 commits into
base: master
Choose a base branch
from
Open

AFK players handling and diverse modifications #17

wants to merge 57 commits into from

Conversation

Doezer
Copy link

@Doezer Doezer commented Mar 2, 2017

  • AFK handling: the bot now handles users who go AFK (either after user inactivity time or by manually going into idle mode). Users are removed from the current queue and added to a temporary AFK queue until they come back from AFK, in which case they are removed from the temporary queue and added back to the ongoing queue. A new organiser is used to do this.
  • Features toggling : added a system to toggle features (tried to it so that more features can be added)
  • Players announcement: AFK players are also announced the same way as the others, with the slight modification of not showing current queue count (useless).
  • Playerrs announcement: the announcement is not made anymore after a player goes offline/AFK/online-from-AFK. Current queue count is sufficient in my mind.
  • Players announcement: Players are displayed using "\n" instead of ",". Bit more noisy but I think it adds lisibility. Also, given previous change, the noisiness is relative.
  • Added "botname" to the JSON file. Just for dev tests to know which bot is being used.
  • Added mentions instead of displaying simply the author's name, except for AFK messages which were "waking up" the user being mentioned and making it usually come back from AFK, rendering the whole thing pretty noisy. Still being field-tested though.
  • If the user types !add but was already added, displays a message saying it
  • Added is_not_ready method for queues
  • Added commands aliases to doc
  • Modified current unit tests to pass Travis checks

@coveralls
Copy link

Coverage Status

Coverage decreased (-7.4%) to 52.514% when pulling 0c598c4 on Doezer:english into d51183b on veryhappythings:master.

@veryhappythings
Copy link
Owner

veryhappythings commented Mar 3, 2017

Hi, just wanted to say thanks for this - I'm very busy at the moment but I'll try to look at it ASAP :)

* path of config file in variable for easier testing
added test cases for main
added test case for organiser.is_not_ready()

* path of config file in variable for easier testing
added test cases for main
added test case for organiser.is_not_ready()

* bump

* bump
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.8%) to 53.073% when pulling d427bdb on Doezer:english into d51183b on veryhappythings:master.

@Doezer
Copy link
Author

Doezer commented Mar 4, 2017

Disregard the commit messages, I first added test cases for main only to see that it was too complicated (or impossible I don't know)

A few fixes and a new test case for is_not_ready, that's all

@coveralls
Copy link

Coverage Status

Coverage decreased (-6.8%) to 53.073% when pulling 73bae42 on Doezer:english into d51183b on veryhappythings:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-7.09%) to 52.778% when pulling 52c5483 on Doezer:english into d51183b on veryhappythings:master.

Added message if not admin for reset and toggle
Added support for toggable features
Added json config file support to the toggle command so that the command status is saved when bot is OFF
Allowed arguments for toggle command
Removed temporarily unit tests for Gatherbot
@coveralls
Copy link

Coverage Status

Coverage decreased (-21.4%) to 38.462% when pulling 5be4744 on Doezer:english into d51183b on veryhappythings:master.

@Doezer
Copy link
Author

Doezer commented Mar 4, 2017

Sorry for the supplementary review work ! I'm sure I made mistakes and had to decrease coverage because I didn't try to use the mock module. I will try and do it tomorrow.

@coveralls
Copy link

Coverage Status

Coverage decreased (-21.4%) to 38.462% when pulling a4e5c4e on Doezer:english into d51183b on veryhappythings:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-21.2%) to 38.627% when pulling 4a229eb on Doezer:english into d51183b on veryhappythings:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-22.2%) to 37.657% when pulling 436d053 on Doezer:english into d51183b on veryhappythings:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-22.4%) to 37.5% when pulling 436d053 on Doezer:english into d51183b on veryhappythings:master.

* path of config file in variable for easier testing
added test cases for main
added test case for organiser.is_not_ready()

* path of config file in variable for easier testing
added test cases for main
added test case for organiser.is_not_ready()

* bump

* bump

* Added toggle cmd to toggle features on/off
Added message if not admin for reset and toggle

* added support for toggable features

* Added json config file support to the toggle command so that the command status is saved when bot is OFF

* Added json support to features handling

* Allowed arguments for toggle command

* Allowed arguments for toggle command

* small fix

* bump

* Removed temporarily unit tests for Gatherbot

* bump

* bump

* bump

* bump

* bump

* bump

* bump

* bump

* bump

* bump

* bump

* bump

* Adds the field to json if not already created

* Adds the field to json if not already created

* readme

* readme

* Added premade feature

* turned off mail notifications

* removed useless import

* updated tests

* bump

* bump

* Internalization for English and French DONE for strings and docstrings. Probably messy, but it works
Added !language command to modify during runtime
Also present in json config file for manual modification
if no locale is present in json config file at startup, default is locale of environment. if not english or french, then it's english.

messages.pot to be used for localization.

* Internalization for English and French DONE for strings and docstrings. Probably messy, but it works
Added !language command to modify during runtime
Also present in json config file for manual modification
if no locale is present in json config file at startup, default is locale of environment. if not english or french, then it's english.

messages.pot to be used for localization.

* Internalization for English and French DONE for strings and docstrings. Probably messy, but it works
Added !language command to modify during runtime
Also present in json config file for manual modification
if no locale is present in json config file at startup, default is locale of environment. if not english or french, then it's english.

messages.pot to be used for localization.
@Doezer
Copy link
Author

Doezer commented Mar 5, 2017

Back for more : added internalization to the package.

@veryhappythings
Copy link
Owner

Hi! Just found the time to take a look through this. Thanks for all the work, it's great :)

Unfortunately I can't easily merge this PR because it adds a number of features all bundled together, so it's hard for me to evaluate each one. I will go through them and add them, though! I might change a few things to suit my use cases but I think we both want the same things.

Thanks again, this is a great help. I'm amazed that you've gone ahead and localized it - that's fantastic!

@Doezer
Copy link
Author

Doezer commented Mar 13, 2017

Hello,
I understand that this isn't mergeable "as is". I don't know if there's any way to make like multiple pull requests since I do it in Pycharm and it gave me an error when I tried to open a second one.

Let me know if I can help.

@veryhappythings
Copy link
Owner

Generally you'd write the features on different branches and open a separate PR for each one. That's easy if you're not actively using a project, but as this is in use by you right now, it's tricky to do. Thanks for the offer, I think I just need to double check that all of this is something I want in the project and add it in. I'd have done it this weekend but I've had some really important projects going on - they should be coming to an end today so I'll get back to this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants