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

#1984 (texto en visual objects) y #1985 (objetos invisibles) #1986

Merged
merged 2 commits into from
Feb 15, 2021

Conversation

lgassman
Copy link
Contributor

Definiendo un método el método text() en un visual object se agrega el texto por encima de la imagen. El color por default es Azul, pero se puede definir el metodo textColor() para redefinirlo. textColor debe devolver un string con un valor RGBA en hexa: "rrggbbaa" https://libgdx.badlogicgames.com/ci/nightlies/docs/api/com/badlogic/gdx/graphics/Color.html.

En el momento de la construcción se determina si el objecto entiende esos mensajes o no para saber si en cada ciclo se debe dibujar el texto o no.

Con este requirimiento la presencia del método image() pasa a ser opcional, ya que se puede querer un objeto visual que sea solo texto. Se cambió el comportamiento para que no se dibuje la imagen default en caso de que no se defina el método image()

Si no existe ni el método text, ni el método image entonces el objeto es invisible y se puede utilizar para disparar colisiones.

@coveralls
Copy link

coveralls commented Dec 27, 2020

Coverage Status

Coverage increased (+0.09%) to 83.932% when pulling 89b20ca on add_#1984_#1985_text_on_visual_objects into 0ac87bb on dev.

@lgassman lgassman force-pushed the add_#1984_#1985_text_on_visual_objects branch from d4f7b89 to 8deb94b Compare December 27, 2020 23:41
@lgassman lgassman force-pushed the add_#1984_#1985_text_on_visual_objects branch from 8deb94b to 20095de Compare December 27, 2020 23:48
Comment on lines 88 to 90
message.hasProperty && parameters.length() <= 1 ||
message.hasConstantProperty && parameters.empty

Copy link
Collaborator

Choose a reason for hiding this comment

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

ésta es la única parte que me hace ruido, me gustaría tener un método de alto nivel que exprese "si un objeto tiene una propiedad independientemente de si es constante o variable" y que este método lo llame. En realidad habría que preguntar (message.hasGetter && parameters.empty) || (message.hasSetter && parameters.length === 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eso es medio dificil, porque un getter puede ser debido a una property o porque escribieron el method a mano. Si lo llevo por el lado del hasGetter tendría que o bien volver a ejecutar el method lookup, o bien hacer una implementación "incompleta" que en realidad se fije si tiene un getter definido por una property.
Lo que te puedo ofrecer es renombrar el hasProperty por hasVarProperty, ya que eso es lo que realmente significa ese método. Luego un método que sea canBeHandledByProperty(message, parameter) -o el nombre que te guste- que tenga como cuerpo message.hasVarProperty && parameters.length() <= 1 || message.hasConstantProperty && parameters.empty. Eso encapsula un poco más los detalles de implementación y te da el concepto de si algo es manejado por una property independientemente si es var o const. No se si haría tanto lío la verdad. Creo que para librarse de tener que andar preguntando si es const property o property el lugar que habría que tocar es donde se arma el ast(?) para que cuando se parsee el property se generen métodos de verdad que sean encontrados por el lookup. Eso sería más complejo (no me animo a meterme en eso)

A mi en realidad lo que me hace ruido de esto es la lista de parámetros. Sospecho que en realidad no se necesita tener una lista de parámetros si no solamente saber la cantidad de parámetros. Tener la lista te deja a futuro jugar con la posibilidad de multimethods, pero también te quita la posibilidad de usar este método más estáticamente. Yo safé porque estamos usando métodos que no tienen parámetros, pero si el método que tuviera que saber si entiende fuera text(anchoMaximo) tendría que inventar una lista de parametros que no existen aún.

Copy link
Collaborator

Choose a reason for hiding this comment

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

sí, hasVarProperty me convence.

Cambiar el AST coincido en que no es el approach más copado para hacerlo, al menos Xtext no está pensado para eso (quiere más extension methods sobre una sintaxis que puede modificarse).

Copy link
Collaborator

@fdodino fdodino left a comment

Choose a reason for hiding this comment

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

Me parece que es una funcionalidad piola, hasta ahora vi que muchos en los juegos armaban imágenes con las letras, a futuro podríamos soportar diferentes tipos de fuente (typeface). Me suena a que es algo interesante para explorar.

Me hubiera gustado verlo en acción con un gif, no se Nahue (@PalumboN) qué te parece, si lo charlaste con Leo en UNQ.

@lgassman
Copy link
Contributor Author

lgassman commented Jan 6, 2021

Me parece que es una funcionalidad piola, hasta ahora vi que muchos en los juegos armaban imágenes con las letras, a futuro podríamos soportar diferentes tipos de fuente (typeface). Me suena a que es algo interesante para explorar.

La fuente es fácil también de hacerlo, pero me pareció un poco excesivo llenar el visualObject con tantos métodos. También se podría jugar con la alineación, o los márgenes (que están medio hardcoded por ahí). No se si vale la pena ahora.

Me hubiera gustado verlo en acción con un gif, no se Nahue (@PalumboN) qué te parece, si lo charlaste con Leo en UNQ.

Si me mandás un link que diga como se hacen esos gifs te hago uno muy bonito

Copy link
Collaborator

@fdodino fdodino left a comment

Choose a reason for hiding this comment

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

para hacer gifs, en Linux tenés Peek, o Silentcast.
En Windows https://screencast-o-matic.com/screen-recorder, una vez que te genera el gif lo pegás acá mismo y te queda attachado.
Abrazo

Comment on lines 88 to 90
message.hasProperty && parameters.length() <= 1 ||
message.hasConstantProperty && parameters.empty

Copy link
Collaborator

Choose a reason for hiding this comment

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

sí, hasVarProperty me convence.

Cambiar el AST coincido en que no es el approach más copado para hacerlo, al menos Xtext no está pensado para eso (quiere más extension methods sobre una sintaxis que puede modificarse).

- #1985 wollok-game: Allow invisible components
@lgassman lgassman force-pushed the add_#1984_#1985_text_on_visual_objects branch from 20095de to 3d9296d Compare January 9, 2021 20:20
@lgassman
Copy link
Contributor Author

lgassman commented Jan 9, 2021

texto-en-visual-object

@lgassman
Copy link
Contributor Author

lgassman commented Jan 9, 2021

import wollok.game.*

object colores {

	const property verde = "00FF00FF"
	const property rojo = "FF0000FF"

}

//solo imagen
object dodain {

	const property image = "dodain.png"
	var property position = game.at(3, 3)

}

//texto, imagen y color
object nahuel {

	const property image = "nahuel.png"
	var property position = game.at(7, 3)
	var visitante = null

	method visitadoPor(alguien) {
		visitante = alguien
	}

	method visitado() {
		return visitante != null
	}

	method text() {
		return if (self.visitado()) "¡Qué piola! ¡Vino " + visitante.toString() + "!" else "¡Qué bajón!"
	}

	method textColor() {
		return if (self.visitado()) colores.verde() else colores.rojo()
	}

}

object soloTexto {

	const property position = game.at(5, 9)
	const property text = "¿A dónde vas Dodain?"

}

object invisible {

	const property position = game.at(5, 3)

}

program programa {
	game.height(10)
	game.width(10)
	game.addVisualCharacter(dodain)
	game.addVisual(nahuel)
	game.addVisual(soloTexto)
	game.addVisual(invisible)
	game.onCollideDo(invisible, { alguien => game.say(alguien, "¿Qué fue eso?") })
	game.onCollideDo(nahuel, { alguien => nahuel.visitadoPor(alguien) })
	game.start()
}

@lgassman
Copy link
Contributor Author

lgassman commented Jan 9, 2021

Ahi hice lo de la varProperty. Resulta que la colección de properties tenía todas las properties (const y var) y luego habia una coleccion con solo las const. Así que lo que hice fue hacer también la colección que tiene solo las var. Un poco me jode que la coleccion de properties podría ser calculada a partir de las otras dos. Pero como es algo que se invoca mucho lo dejé así. Por otro lado hice también el programa de ejemplo, con el gif, y también le metí un test case para los cambios en wollokObject.

@lgassman lgassman requested a review from fdodino January 9, 2021 21:48
Copy link
Collaborator

@fdodino fdodino left a comment

Choose a reason for hiding this comment

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

Con los últimos cambios quedó perfecto.

@fdodino fdodino merged commit e7bded4 into dev Feb 15, 2021
@fdodino fdodino deleted the add_#1984_#1985_text_on_visual_objects branch February 15, 2021 12:09
@PalumboN PalumboN mentioned this pull request Mar 9, 2021
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.

Bueno, llegué 2 meses tarde pero llegué. GROSOOOO!!!!! LA GENTE VA A ESTAR MUY CONTENTA CON ESTE CAMBIO :D

Ahí dejé un comentario sobre un bug que quedó y ya tiré un PR para eso.

@lgassman ahora que estás más metido en WG quizá quieras ver este issue con una larga discusión #1319 por si tenés algo para aportar (puede que no, solo lo tiro porque lo tenía a mano :P)
Y después quiero pensar un toque este: #1946 que está pelado de discusiones.

Comment on lines 30 to 40
new(WollokObject object) {
wObject = object
wObject?.position // Force evaluate position when is added
this(object, object.position)
}

new(WollokObject object, WollokObject position) {
wObject = object
wPosition = position
hasText = wObject.understands(TEXT_CONVENTION)
hasTextColor = wObject.understands(TEXT_COLOR_CONVENTION)
hasImage = wObject.understands(IMAGE_CONVENTION)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hay que tener cuidado con esto. Este objeto tiene 2 constructores por las dos formas que tiene WG de indicar la posición de un objeto:

  • game.addVisual usa el primero, e indica que el objeto es quien conoce a su posición.
  • game.addVisualIn la posición es pasada por parámetro y el objeto no la conoce.

Si hacemos que uno delegue en el otro, CREO que el primer (y más usado) caso cachearía la posición inicial pero no se actualizaría a medida que el objeto cambie su posición.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sí, efectivamente acá había quedado ese bug, ahí lo arreglé en #1990 junto con otros detalles consecuencias de este PR) 😃

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

Successfully merging this pull request may close these issues.

Agregar Componentes Invisibles Agregar Texto en los componentes visuales
4 participants