-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix game paths #85
Fix game paths #85
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
te dejé comentarios, por ahora de newbie, necesito hacer más gimnasia para revisar a conciencia estos PRs. La radio está buenísima!!
src/commands/run.ts
Outdated
@@ -108,6 +112,8 @@ export default async function (programFQN: Name, { project, skipValidations, por | |||
socket.on('keyPressed', key => { | |||
queueEvent(interp, buildKeyPressEvent(interp, wKeyCode(key.key, key.keyCode)), buildKeyPressEvent(interp, 'ANY')) | |||
}) | |||
|
|||
if (!folderImages) logger.warn(failureDescription('Folder for assets not found!')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acá no la tengo tan clara como para ver cómo funciona un programa con Wollok Game. Pero quizás estaría bueno diferenciar:
- ejecutar un programa común
- ejecutar un Wollok Game, donde yo directamente rompería si no tenés una carpeta assets (mientras que en el primer caso no lo necesitamos)
Igual no tengo una opinión muy fuerte, solo tengo el sentimiento de que ese logger.warn
puede llegar a ser pasado por alto si no somos explícitos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eso lo discutimos con Ivo. Por ahora toma como carpeta el base del proyecto.
Lo podemos retomar cuando podamos ejecutar un programa común sin ruido #86
} | ||
}) | ||
const baseFolder = folderImages ?? pathProject | ||
const loadImagesIn = (basePath: string) => fs.readdirSync(basePath, { withFileTypes: true }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uh, me hiciste buscar a ver si hay alguna alternativa de readdirSync
que me permita obtener todos los archivos recursivamente, para poder hacer un map
y meterlo directamente en la const images
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pero bueno... por ahora puedo bajar la manija y hacerme el gil. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jajaja lo buscamos y no lo encontramos 😞
Fix #53
Fix #61
package.json
(nos quedamos con eso o sugieren otro nombre?)"subfolder/image.png"
.w/ @ivojawer
TODO: Tener algunos tests con un proyecto de ejemplo. Testear el server?
Hay otro TODO más grande para revisar / mejorar