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

Redesign #21

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@NeroReflex
Owner

NeroReflex commented Aug 9, 2017

I plugin possono catturare gli eventi.

NeroReflex added some commits Aug 9, 2017

@NeroReflex

This comment has been minimized.

Show comment
Hide comment
@NeroReflex

NeroReflex Aug 9, 2017

Owner

Queste modifiche sono molto importanti e riguardano il modo nei quali i plugin vengono chiamati a comportarsi:

  • ho aggiunto delle funzioni che i plugin possono implementare per catturare gli eventi (onUserEnterChanel, onUserLeaveChanel ecc..)
  • ho cambiato la firma della funzione onCall il cui utilizzo adesso è più immediato
  • la classe Request (che attivava il metodo onCall con l'ausilio di una coda bloccante) adesso e' diventata la classe Event, che rappresenta un evento al quale ogni plugin deve rispondere
  • ogni tipo di evento (enum EventType) ha la sua classe corrispondente
  • ogni evento viene inviato a tutti i plugin ad eccezione di quello che chiama un singolo plugin all'esecuzione (!calc 3 + 1 è inviato solo al plugin calc)
  • ho modificato la documentazione in accordo ai cambiamenti
  • la classe Trancio (quella base dei plugin) contiene un metodo chiamato "processEvent" che si occupa di chiamare il giusto event handler ad ogni evento ricevuto.
  • onHelp è ora una funzione void che viene eseguita sul thread del plugin, per scongiurare effetti indesiderati e dare maggiore stabilità al thread primario
  • spostato il saluto in un plugin che si occupa di salutare i nuovi utenti
Owner

NeroReflex commented Aug 9, 2017

Queste modifiche sono molto importanti e riguardano il modo nei quali i plugin vengono chiamati a comportarsi:

  • ho aggiunto delle funzioni che i plugin possono implementare per catturare gli eventi (onUserEnterChanel, onUserLeaveChanel ecc..)
  • ho cambiato la firma della funzione onCall il cui utilizzo adesso è più immediato
  • la classe Request (che attivava il metodo onCall con l'ausilio di una coda bloccante) adesso e' diventata la classe Event, che rappresenta un evento al quale ogni plugin deve rispondere
  • ogni tipo di evento (enum EventType) ha la sua classe corrispondente
  • ogni evento viene inviato a tutti i plugin ad eccezione di quello che chiama un singolo plugin all'esecuzione (!calc 3 + 1 è inviato solo al plugin calc)
  • ho modificato la documentazione in accordo ai cambiamenti
  • la classe Trancio (quella base dei plugin) contiene un metodo chiamato "processEvent" che si occupa di chiamare il giusto event handler ad ogni evento ricevuto.
  • onHelp è ora una funzione void che viene eseguita sul thread del plugin, per scongiurare effetti indesiderati e dare maggiore stabilità al thread primario
  • spostato il saluto in un plugin che si occupa di salutare i nuovi utenti

@doppioandante doppioandante self-requested a review Aug 9, 2017

@doppioandante

In generale OK l'interfaccia pubblica di Trancio, invece non mi convince la struttura delle classi Event perché avere EventType e anche il subclassing va contro l'open/closed principle (e tuttavia in questo caso non vogliamo che la classe Event sia estendibile se non da noi).

Show outdated Hide outdated src/main/java/com/neroreflex/pizza/Pizza.java
Vedere il [Javadoc](javadoc.md) per la documentazione sulla classe Request.
- protected void onInitialize(...): metodo chiamato nel main thread, al momento della attivazione del plugin, quindi _*PERICOLOSO*_
- protected void onShutdown(...): metodo chiamato nel main thread, al momento della attivazione del plugin, quindi _*PERICOLOSO*_

This comment has been minimized.

@doppioandante

doppioandante Aug 14, 2017

Collaborator

Cosa vuol dire tecnicamente pericoloso?

@doppioandante

doppioandante Aug 14, 2017

Collaborator

Cosa vuol dire tecnicamente pericoloso?

This comment has been minimized.

@NeroReflex

NeroReflex Aug 15, 2017

Owner

Una divisione per zero blocca l'intero bot, non solo il thread

@NeroReflex

NeroReflex Aug 15, 2017

Owner

Una divisione per zero blocca l'intero bot, non solo il thread

This comment has been minimized.

@NeroReflex

NeroReflex Aug 15, 2017

Owner

Beh la divisione per zero era un esempio... Una eccezione non gestita causa il blocco del thread... Ma quelle funzioni sono eseguite sul thread principale...

@NeroReflex

NeroReflex Aug 15, 2017

Owner

Beh la divisione per zero era un esempio... Una eccezione non gestita causa il blocco del thread... Ma quelle funzioni sono eseguite sul thread principale...

This comment has been minimized.

@doppioandante

doppioandante Aug 16, 2017

Collaborator

SI potrebbero fare eseguire sul thread del plugin, ma per ora lasciamo così.

@doppioandante

doppioandante Aug 16, 2017

Collaborator

SI potrebbero fare eseguire sul thread del plugin, ma per ora lasciamo così.

Show outdated Hide outdated src/main/java/com/neroreflex/plugins/Hello.java
Show outdated Hide outdated src/main/java/com/neroreflex/pizza/Trancio.java
*
* Questa funzione e' chiamata in un thread diverso di quello che chiama
* unqueueRequest, precisamente dal thread principale del bot.
*
* Se unqueueRequest ed enqueueRequest dovessero essere synchronized
* Se unqueueEvent ed enqueueEvent dovessero essere entrambi synchronized

This comment has been minimized.

@doppioandante

doppioandante Aug 14, 2017

Collaborator

dovessero

@doppioandante

doppioandante Aug 14, 2017

Collaborator

dovessero

This comment has been minimized.

@NeroReflex

NeroReflex Aug 15, 2017

Owner

Che c'è che non va? Ho scritto dovessero.

@NeroReflex

NeroReflex Aug 15, 2017

Owner

Che c'è che non va? Ho scritto dovessero.

This comment has been minimized.

@doppioandante

doppioandante Aug 15, 2017

Collaborator

Ero convinto di aver letto dovrebbero, devo mettere gli occhiali

@doppioandante

doppioandante Aug 15, 2017

Collaborator

Ero convinto di aver letto dovrebbero, devo mettere gli occhiali

Show outdated Hide outdated src/main/java/com/neroreflex/plugins/Time.java
Show outdated Hide outdated src/main/java/com/neroreflex/plugins/UserCounter.java
Show outdated Hide outdated src/main/java/com/neroreflex/pizza/Event.java
Show outdated Hide outdated src/main/java/com/neroreflex/pizza/Pizza.java
Show outdated Hide outdated src/main/java/com/neroreflex/pizza/Pizza.java

NeroReflex added some commits Aug 15, 2017

@doppioandante

Ora mi piace molto di più, c'è ancora qualche cosa qua e là da sistemare.

Vedere il [Javadoc](javadoc.md) per la documentazione sulla classe Request.
- protected void onInitialize(...): metodo chiamato nel main thread, al momento della attivazione del plugin, quindi _*PERICOLOSO*_
- protected void onShutdown(...): metodo chiamato nel main thread, al momento della attivazione del plugin, quindi _*PERICOLOSO*_

This comment has been minimized.

@doppioandante

doppioandante Aug 16, 2017

Collaborator

SI potrebbero fare eseguire sul thread del plugin, ma per ora lasciamo così.

@doppioandante

doppioandante Aug 16, 2017

Collaborator

SI potrebbero fare eseguire sul thread del plugin, ma per ora lasciamo così.

Show outdated Hide outdated src/main/java/com/neroreflex/pizza/Pizza.java
Show outdated Hide outdated src/main/java/com/neroreflex/plugins/Time.java
Show outdated Hide outdated src/main/java/com/neroreflex/plugins/HackerNews.java
Show outdated Hide outdated src/main/java/com/neroreflex/plugins/Convert.java
Show outdated Hide outdated src/main/java/com/neroreflex/pizza/Trancio.java
Show outdated Hide outdated src/main/java/com/neroreflex/plugins/PTforum.java

NeroReflex added some commits Aug 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment