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

#17 Primera implementación del BAN-HAMMER #22

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

#17 Primera implementación del BAN-HAMMER #22

wants to merge 24 commits into from

Conversation

Santixs
Copy link
Contributor

@Santixs Santixs commented Aug 17, 2020

Implementación que borra un mensaje cuando contiene una palabra de la lista de palabras prohibidas, y después avisa por privado al autor de cual es la palabra o palabras que no puede poner.

También permite a las personas con el rol de junta, añadir y quitar palabras de dicha lista.

@Santixs Santixs changed the title #17 #17 Primera implementación del BAN-HAMMER Aug 17, 2020
@JustAntoRS
Copy link
Contributor

Mismo comentario que a Carlos, se nos esta yendo de las manos el trabajar en bot.py y al final bot.py va a tener 5000 lineas y va a ser imposible de mantener.

Echale un ojo a como mover la funcionalidad a otro fichero y que bot.py solo llame funciones, o que importe o algo asi. Para asi tener cada funcionalidad en un fichero distinto y sera mucho mas facil de mantener y modificar en el futuro

@jonsalchichonnn
Copy link
Contributor

@Santixs Puedo hacer lo de cogs si quieres que hoy estoy con ganas

@jonsalchichonnn
Copy link
Contributor

Ya está hecho, pero no sé cómo hacer commit a la branch :(

# If one with permitted roles is baning a word, we dont ban his message
if have_permitted_rol(message.author.roles) and str_[0] == self.bot.command_prefix \
and str_[1:] in censor_command_names:
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creo que el pass sobra, yo pondría:
if not(have_permitted_rol(message.author.roles) and str_[0] == self.bot.command_prefix
and str_[1:] in censor_command_names):
await message.delete() # Delete the message
# Send a message on the same channel
await message.channel.send(message.author.mention + ", debes cuidar tu vocabulario, jovencito")
# Send a private message to the user
await message.author.send("El mensaje \n" + str(f'diff\n-"{message_content}"') +
"no se ajusta a las normas, en los próximos mensajes no uses: " +
str(forbidden_words_used).strip('[]'))

@JustAntoRS
Copy link
Contributor

Como comentario general, veo muchas clases en un mismo archivo, a mi me gusta mas tener una clase por archivo pero podemos debatirlo, alguno tiene una opinion respecto a esto?

Por lo demas esto ya estaria verdad? Faltaria hacer pruebas un dia para que el bot no sea muy estricto y ver como podemos mejorarlo y seguir iterando. Creo que es una feature muy guay que podemos iterar mucho en ella.

@Santixs
Copy link
Contributor Author

Santixs commented Sep 11, 2020

Como comentario general, veo muchas clases en un mismo archivo, a mi me gusta mas tener una clase por archivo pero podemos debatirlo, alguno tiene una opinion respecto a esto?

Por lo demas esto ya estaria verdad? Faltaria hacer pruebas un dia para que el bot no sea muy estricto y ver como podemos mejorarlo y seguir iterando. Creo que es una feature muy guay que podemos iterar mucho en ella.

Respecto lo de las clases como querais, por mi no hay problema en hacerlo de una manera o de otra.

He implementado un sequence matcher para que si se cambia alguna letra también detecte la palabra, además he quitado palabras repetidas o demasiado parecidas de la blacklist.

Ahora mismo tienen que tener las palabras más de un 80% de coincidencias, con ese valor, en las pruebas que he hecho me ha dado buenos resultados por lo que estaría listo para ponerlo en un entorno real y ya ahí terminar de ajustar el porcentaje.

Copy link
Contributor

@JustAntoRS JustAntoRS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

La carpeta /__pychache__/ deberia estar dentro de .gitignore, estas subiendo al repositorio archivos binarios de Python :P

Otro comentario es lo de clases, el archivo ban_hammer.py es demsiado grande, prefiero 1 archivo por clase la verdad (estoy haciendo este comentario a varios mas)

Comment on lines 109 to 111
await message.author.send("El mensaje \n" + str(f'```diff\n-"{message.content}"```') +
"no se ajusta a las normas, en los próximos mensajes no uses: " +
str(forbidden_words_used).strip('[]') + "ni parecidos.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
await message.author.send("El mensaje \n" + str(f'```diff\n-"{message.content}"```') +
"no se ajusta a las normas, en los próximos mensajes no uses: " +
str(forbidden_words_used).strip('[]') + "ni parecidos.")
msg = "El mensaje\n{} no se ajusta a las normas, en los proximos mensajes no uses: {} ni parecidos".format( str(f'```diff\n-"{message.content}"```'), str(forbidden_words_used).strip('[]'))
await message.authord.send(msg)

En vez de sumar strings podrias usar alguna de las opciones de Python para formatear strings, se entienden mejor al final, puedes aplicar esto a todos los strings, que hay varios mas que construyes haciendo sumas.

Copy link
Contributor

@JustAntoRS JustAntoRS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me falta documentacion en las funciones, como minimo de que tipo deberia ser cada argumento y que representa.

@JustAntoRS JustAntoRS self-requested a review September 19, 2020 20:18
@JustAntoRS JustAntoRS linked an issue Sep 19, 2020 that may be closed by this pull request
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.

Funcionalidad 10: Ban Hammer automatico (el mejor ban hammer)
4 participants