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

Invisible objects and objects with text #69

Merged
merged 17 commits into from
Aug 27, 2021
Merged

Invisible objects and objects with text #69

merged 17 commits into from
Aug 27, 2021

Conversation

AngeliMatias
Copy link
Contributor

Resolved #55 based on uqbar-project/wollok#1986

@ezequielPereyra
Copy link
Contributor

Así se vería ahora.

dodain_meets_rasta

Acá hay un repo que tiene el ejemplo.

Copy link
Member

@nscarcella nscarcella 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 contexto para opinar sobre el feature y no conozco cuál es el caso de uso, pero el código parece razonable.

Por mi está OK, aunque yo tenía entendido que eso de que te falle si no le ponés una imagen era apropósito, porque así, si el pibe se olvidaba de ponerlo se lo recordaba (cosa que me parece más frecuente que querer componentes invisibles).

Capaz sería mejor defaultear a que si no tenés imagen siga explotando y darles alguna facilidad para poder crear una imagen transparente, cosa de que tengan que declararlo explicitamente, onda:

method image = game.transparent

Si la imagen es transparente la position medio que no importa, con lo cual podríamos seguir requiriendoles ponerla así no hay dudas de dónde dibujar los mensajes.

@lspigariol
Copy link

lspigariol commented Aug 18, 2021 via email

@ezequielPereyra
Copy link
Contributor

ezequielPereyra commented Aug 19, 2021

¡Buenas! Creo que dimos por sentado muchas cosas y no explicamos bien lo que estuvimos haciendo. Nuestro objetivo al crear este PR era implementar un feature ya existente en wollok xtext 3. Es decir, ya se había tomado la decisión de que los visuales pudieran no tener imagen (por lo que ahora serían más posicionables que visuales). Los features basados en este PR son los siguientes:

  1. Poder definir visuales que muestren un texto por encima de la imagen. Esto se logra definiendo el método text() que devuelva un string con el mensaje a mostrar. Esto es opcional.
  2. Poder cambiar el color del texto de manera opcional. Por default quedó en que sea azul, pero se puede cambiar definiendo el método textColor() que devuelva un string con el valor en hexadecimal del color RGBA.
  3. Como puede suceder que se quiera definir un visual que solamente represente un texto, se decidió que la imagen sea opcional.

Así se ve en wollok xtext 3:

104109634-c3efec80-52ae-11eb-9fbc-8038b7e09a5c

Y así se ve en wollok ts con los cambios que hicimos:

ts129293724-0de8aa24-9627-482e-990a-5a9eb145d3fb


Como dijo Lucas, un caso de uso posible es querer extender la superficie colisionable de un objeto, así ocuparía más de una celda.

Ahora mismo no estoy seguro de qué pasaba si el visual no tenía imagen. Me acuerdo que si el path estaba mal o no la encontraba mostraba el logo de wollok. No recuerdo si explotaba.

Si quisiéramos que la imagen sea obligatoria tendríamos que hacer cambios en wollok xtext también. Y ya que estamos quizás esté bueno debatir la idea de reificarla. Se esa manera podríamos tener algo así como su opacidad, lo que permitiría que el objeto sea invisible. U otras cosas de interés, como la capacidad de cambiar su tamaño, espejarla, rotarla, etc. Lo mismo podría ir para el texto. Si lo reificamos podríamos agregar varias características más, como el tamaño de la letra, la fuente, si va en negrita, entre otras cosas. Aunque también se podría seguir por el camino de agregar métodos opcionales que las definan.

@ezequielPereyra
Copy link
Contributor

@PalumboN Cuando se define una imagen que no se encuentra se muestra directamente el loguito de wko de wollok. Un poco diferente a xtext que te pone un texto encima que dice image not found. Adjunto imagenes:

En xtext:

image

En ts:

image

Se podría cambiar la imagen pero no encuentro la fotito del wko. Y no sé bien dónde debería agregar la nueva para que se vea como en xtext.

@AngeliMatias
Copy link
Contributor Author

AngeliMatias commented Aug 21, 2021

Hice esta imagen para el loguito de imagen no encontrada:
wko

Momentaneamente la dejé en la carpeta public, después busco bien como agregarla para que utilice esta como default

Copy link
Contributor

@PalumboN PalumboN left a comment

Choose a reason for hiding this comment

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

Hola a tod@s! Dejo algunos comentarios sobre lo que se está haciendo acá:

  • Como decían, el mensaje image siempre fue opcional, y lo que pasaba antes era que te dibujaba el logo de Wollok. Esto fue pensado como la imagen default de cualquier objeto, y servía para una estrategia didáctica incremental donde el objeto que ya tenés codeado lo metés al tablero y de pronto se ve (por eso queríamos que se mostrara de alguna manera y no dar la sensación de que no está pasando nada).
  • Lo que terminó pasando es que la forma en que introducimos WG en clase nunca se hacía uso de este feature. Ya el primer ejemplo está pensado como un juego y ya tiene una pepita con una imagen. De hecho, en el enunciado incremental de WG que estamos diseñando ya comienza con un juego bastante armado que hay que extender.
  • Por otro lado, al meter el TP integrador del juego y crear juegos sarpados, a veces les estudiantes necesitaban tener objetos invisibles (por distintos motivos, como las colisiones que comentaban), y era tedioso lograrlo.
  • Pero más tedioso era meter textos, donde sí o sí tenían que pasar por una imagen y jugar con eso. Y este es el principal problema que buscaba arreglar Leo en el PR de Xtext.
  • Al meter la posibilidad de tener "objetos textuales", lo más probable es que muchas veces no vamos a querer una imagen para estos textos, o sea que habría que configurarlos transparentes. Por eso se decidió cambiar el default de la imagen, y para mí es lo que queremos.

Por otro lado, sí se contempla error cuando tenés un método image() que devuelve un path que no existe, porque posiblemente le estés pifiando a algo, y como siempre tratamos de ejecutar el juego por más que tenga errores, decidimos mostrar el logo de Wollok con el texto de que "no se encontró la imagen".
Esto estaría bueno tenerlo, @ezequielPereyra @AngeliMatias. No sé si quieren hacerlo acá o lo dejamos para otro PR, si quieren cargamos el issue, como prefieran
Dejo data sobre esto:

  • Acá está la lógica que busca la imagen a partir del texto y si no existe le pone la de wko.png.
  • Esa imagen se encuentra en el repo de Xtext por ahora, pero debería estar en language. Acá es donde se trae las imágenes defaults.
  • Si quieren podemos meter la de "Image not found" ahí y usarse, pero en Xtext ese texto se escribe, no forma parte de la imagen :P Si podemos hacer eso genial, me gusta más, sino vamos por la imagen.

Muy buen trabajo gente!! 🚀

Alto PR se hicieron, y tuvieron que meter mano. Les dejo algunos comentarios 👍

src/components/game/Sketch.tsx Outdated Show resolved Hide resolved
src/components/game/Sketch.tsx Show resolved Hide resolved
src/components/game/Sketch.tsx Outdated Show resolved Hide resolved
src/components/game/SketchUtils.ts Outdated Show resolved Hide resolved
src/test/game.test.ts Outdated Show resolved Hide resolved
src/components/game/Sketch.tsx Show resolved Hide resolved
Comment on lines 153 to 158
try {
gameTest('incomplete visual', 'incompleteVisual', ['games/incompleteVisual.wpgm'], function (interpreter) {})
}
catch(exception) {
expect(exception).toBeInstanceOf(WollokException)
}
Copy link
Contributor Author

@AngeliMatias AngeliMatias Aug 23, 2021

Choose a reason for hiding this comment

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

@PalumboN acá nos encontramos con un problemita: Al intentar agregar el visual que explota, explota directamente gameTest, y no llega a pasar a un getVisualState como en los otros. Para agarrar estos casos lo hicimos de esta manera, pero es algo hiper genérico. No se puede llegar a una position sin definir porque es un TypeError. En lang ya hay un test donde se intenta agregar un visual sin position.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh piola, si eso ya está testeado allá entonces no hay problema. Lo que estaría bueno testear es qué pasa acá con el juego corriendo cuando pasa eso, pero es un test más complicado, lo pensamos después.
Entonces podemos borrar este test y ya estaría para mergear, no?

@PalumboN PalumboN merged commit dab9568 into master Aug 27, 2021
@PalumboN PalumboN deleted the updated-visuals branch August 27, 2021 19:51
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.

Update new visuals Wollok Game
5 participants