fe-martinez / tpAlgo3

2 stars 1 forks source link

Consulta (? #6

Closed claram97 closed 1 year ago

claram97 commented 1 year ago

Buen día profe, entre ayer y hoy estuvimos actualizando el repositorio. Todavía no es una versión final pero estamos muy cerca. Para implementar la interfaz gráfica, creamos clases para cada vista que necesitábamos (una para el menú, una para el juego y una para cada tienda). Las vistas de las Tiendas están recibiendo su Tienda asociada y el jugador, las vistas en este momento son las que se ocupan de que, al seleccionar una opción de la vista, ocurra lo que tenga que ocurrir (vender una mejora, cargar combustible o lo que corresponda ¿Estaría bien esto o qué ajuste habría que hacerle? Desde ya muchas gracias.

claram97 commented 1 year ago

Añado una aclaración. En la clase Interacciones, que maneja, precisamente, las interacciones entre el Jugador y el resto de elementos del Juego, cuando se está en la misma posición que una Tienda, se llama a un método estático de la clase VistasTienda que tiene un map que busca la Tienda que corresponde mostrar.

smaraggi commented 1 year ago

Hola, no puedo ver en el issue de etapa 1 los patrones que iban a implementar, se borraron mensajes o eso quedó anotado en otra parte del repo? Veo solamente que les aprobó la etapa Diego.

Por lo que estuve viendo, las tiendas serían parte del modelo: pertenecen al escenario, tienen una posición en el mapa y ejecutan acciones sobre otras partes del modelo.

Siguiendo el patrón MVC, lo que podrían hacer es aislar un poco más la vista del modelo, agregando una clase "controlador" para cada vista. Estas clases controlador podrían parecer triviales... podría pasar que brinden una interfaz a la vista muy parecida a la que el modelo le ofrezca al controlador, siendo una clase "pasamanos". ¿cuál sería el sentido? Supongan que les dan más presupuesto para extender el proyecto, agregando una interfaz re copada con gráficos geniales y hecha desde cero, tener un controlador que centralice todas las operaciones sobre el modelo facilitaría este proceso.

¿Se justificaría esto en este proyecto? Parcialmente no, porque es un proyecto chico sin vistas a una futura extensión. No obstante, estaría bien para respetar el patrón MVC que la vista no ejecute acciones directamente sobre las clases del modelo, y que poner un controlador se considera una práctica habitual. Si pueden hacerlo, recomendaría meter una clase ControladorX, que se encargue de las tareas sobre el modelo relacionadas a una funcionalidad X (por ejemplo de un tipo de tienda particular). Así tendrían pequeñós controladores para cada tienda.

Quiero ser claro: lo que hicieron no está mal. Para grandes sistemas se estila que se usa el patrón MVC, y como se da en la materia sería buena idea seguir su planteo general. El patrón MVC ayuda a seguir el principio de "Single Responsibility Class"... el controlador opera sobre el modelo y la vista renderiza. Las clasess "controlador" a este efecto deberían ser casi recibir una llamada y llamar a un método muy parecido del modelo (o un par de llamadas correlacionadas, si cabe)". Estas clases "controlador" además pueden facilitar el testing, por ejemplo, sin involucrar vistas.

Cualquier duda consulten de nuevo. Felicitaciones si están por terminar, señal de que vienen bien, ¡sigan adelante!

claram97 commented 1 year ago

Hola! Efectivamente, los patrones se borraron porque estaban en el readme. Sin embargo, recordé que Fran tiene un Notion donde los había guardado, así que los adjunto acá y los vuelvo a poner en el readme:

Patrones propuestos:

Por otro lado, ¿con los sonidos pasa algo parecido no? ¿Hay que separarlos de la vista?

Y la otra pregunta es si tenemos que entregar sí o sí mañana. Creo que Essaya mencionó que no necesariamente tenía que ser mañana. La verdad nos gustaría agregar las clases ControladorX. En cuanto a la funcionalidad del proyecto en sí, ya funciona casi todo. Creo que como medio importante nos falta agregar un par de sonidos y arreglar un bug que hay cuando ganás. La idea igual es ir subiéndolo a medida que vamos avanzando con cosas. Más tarde subiremos la versión actual.

smaraggi commented 1 year ago

No hay una práctica uniforme con el tema de los sonidos. En motores de juegos a veces se estila que los elementos de la escena encargados de reproducirlos tengan una propiedad que administra y ejecuta los sonidos. No obstante, dado que esta es una aplicación con una escena sencilla y de pocos elementos, no tiene tanto sentido que cada cosa tenga una clase y su comportamiento asociado al nivel de un motor de juegos.

Una opción que pueden hacer es que haya una clase encargada de todo el tema de los sonidos. Podría ser un singleton, pero estamos evitando el uso de singletons, así que en su lugar podrían hacer una clase con una instancia creada al momento de levantar la aplicación, y luego pasar esa instancia a las vistas que reproduzcan sonidos, y que estas llamen a un método sin parámetros, por ejemplo:

reproducirSonidoCompraCombustible();

La clase responsable de los sonidos tendría así todos los archivos de sonido y expondría un método para cada caso. Podrían definir además distintas interfaces de sonido (SonidosVistaTaller, por ejemplo) y que la clase de sonidos las implemente todas juntas, o una clase de sonido para cada vista. Finalmente, las vistas podrían reproducir sonidos también, pero capaz les resulte mejor alguno de los métodos anteriores, para separar un poco lo que se ve de lo que se oye.

Tienen que elegir un criterio de diseño para el tema de los sonidos y explicarlo. Yo sacaría el código de sonidos de las clases de vista, si es posible, y que llamen a métodos de reproducir sonidos de afuera de la vista, pero con un método específico para ese sonido (en vez de un método super genérico que reciba por parámetro el archivo de sonido, por ejemplo; yo aislaría el tema de nombres de archivo de sonido de la vista, para que la vista tenga el menor conocimiento posible de los asuntos de sonidos... el día de mañana podría venir otro artista de sonidos y cambiar todos los sonidos manteniendo la interfaz a la vista, por ejemplo, que no cambiaría).

smaraggi commented 1 year ago

Por favor, vuelvan a poner los patrones propuestos en el README del repo. No se por qué o cómo los eliminarojn del issue 1, se supone que los issues son para que quede todo documentado ahí lo que se fue hablando. Gracias por volver a subirlos, ya los anoté, pero déjenlos por favor a la vista en el repo, ya que es lo acordado en la etapa 1.

claram97 commented 1 year ago

Nosotros tenemos una clase así pero estábamos pasando el nombre del sonido por parámetro. ¿Entonces no importa si creamos un montón de métodos iguales que lo único que hagan es cambiar el sonido que se reproduce? Lo pregunto porque va a haber un montón de código largo y repetido. Los patrones ya están en el readme. La verdad es que en el issue 1 nunca pusimos los patrones, los habíamos puesto en el readme, Essaya abrió el issue 1 para aceptar la entrega (ni siquiera sabíamos qué era un issue en ese entonces jajaja) y se nos pasó cuando subimos las actualizaciones y pisamos el readme.

claram97 commented 1 year ago

O sea, en realidad nuestra clase Sonidos tiene un HashMap con los sonidos y se recibe por parámetro la key del sonido buscado, no el nombre del ejecutable. Pero no sé si en vez de esto conviene entonces crear un método para cada sonido.

smaraggi commented 1 year ago

Hola, mantenelo como ya lo tenés hecho.

No está mal para nada.

En general, recibiendo menos parámetros y haciendo más específicos los métodos, más especializada se vuelve la clase que ejecuta los comandos (en este caso la clase de sonidos, que se prepara para reproducir un sonido específico ante un pedido del modelo). Esto favorece desligar a otras clases del modelo de cuestiones de sonido (el nombre del archivo). Pero no le des mayor importancia, está perfecto de las dos maneras.

claram97 commented 1 year ago

Genial, mil graciasss :)