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

Change cell size #1806

Merged
merged 6 commits into from
Nov 21, 2019
Merged

Change cell size #1806

merged 6 commits into from
Nov 21, 2019

Conversation

ivojawer
Copy link
Contributor

Fix #1790

@coveralls
Copy link

coveralls commented Sep 19, 2019

Coverage Status

Coverage decreased (-0.01%) to 89.866% when pulling 21dedf2 on change-cell-size into 59a28dc on dev.

Copy link
Member

@npasserini npasserini left a comment

Choose a reason for hiding this comment

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

Me queda una duda sobre esta propuesta: los juegos en wgame (hasta hoy) muestran imágenes de 50x50, sin escalarlas. Esto es, si yo le pongo una imagen más grande invade las celdas vecinas. Si les pongo imágenes más chiquitas ocupan sólo una parte de la celda.

¿Cómo queda esa característica aquí? Me da la sensación de que va a agrandar las celdas pero no va a escalar las imágenes, entonces el tamaño relativo imagen-celda va a cambiar y el juego se va a ver distinto. Es decir, no puedo usar este feature para ver el juego más grande o más chico en función del tamaño de pantalla disponible porque se va a ver distinto.

No digo que está mal este feature, de hecho tal vez queremos tener tanto esta versión (suponiendo que entendí bien el código) como la que yo propuse. Pero sí me gustaría estar seguro de que nos hicimos la pregunta correspondiente.

Al margen de eso, lo que sí deberíamos hacer es incluir este cambio en el backlog y en la documentación de wgame en el sitio.

@ivojawer
Copy link
Contributor Author

Si, acá solamente se quería cambiar el tamaño de la celda y no escalar el juego entero. Esa opción de escalar el juego me parece excelente pero no es a lo que apuntaba el issue.

@npasserini
Copy link
Member

@ivojawer Che, metemos este cambio en el backlog así lo mergeamos?
@fdodino dice que va a hacer un deploy entre domingo y lunes.

@npasserini
Copy link
Member

@PalumboN Me gustaría también tu ok para mergear esto.

@PalumboN
Copy link
Contributor

Para mí está ok!
@npasserini Esto no escala todo, podríamos abrir otro issue con eso (de paso aprobás este?). Imagino que habrás querido decir CHANGELOG, por lo que veo en https://github.com/uqbar-project/wollok-language/blob/master/CHANGELOG.md faltaría actualizar los cambios de la 1.9.0, no? Esto lo agregamos para la 1.9.1?.
@ivojawer Hay que agregar el cambio en ese archivo (tirar un PR a ese repo).

@npasserini
Copy link
Member

Sí, a full, está repiola el PR.
Sí, CHANGELOG yo decía, todo eso que dijo Nahuel.

En este momento el changelog está mal mal mal, ufa.
Lo que dice "to be released" => tiene que llamarse v1.9 que ya está releaseado.
Y posiblemente falte agregar ahí los ultimos features.

Y habría uqe agregar un nuevo to be released en 1.9.1 que incluya esto.
Podrán?

También habría uqe tocar el sitio incluyendo este feature, podemos pedirle guía a @lspigariol

@PalumboN
Copy link
Contributor

Bueno, ahí me lombardicé: uqbar-project/wollok-language#20 @npasserini

La doc del sitio hay que mirarla toda de nuevo y agregar esto. @ivojawer cargamos un issue con eso y cerramos esto?

@PalumboN
Copy link
Contributor

@npasserini qué hacemos con esto? Ya está para mergear hace un montón y está frenado por cosas de documentación que también hicimos pero no fueron las esperadas... Nos convertimos en el ser que siempre odiamos?

@PalumboN
Copy link
Contributor

@fdodino esto ya está listo para mergear, no sé qué flasheo AppVeyor supongo que hay que restartear el build (ni Ivo ni yo pudimos, creo que nos falta permisos).

@fdodino fdodino merged commit 70b4f49 into dev Nov 21, 2019
@fdodino fdodino deleted the change-cell-size branch November 21, 2019 05:33
@fdodino
Copy link
Collaborator

fdodino commented Nov 21, 2019

Listorti!

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.

Quiero poder cambiar el tamaño de las celdas
5 participants