megamarc / Tilengine

Free 2D graphics engine with raster effects for retro/classic style game development
https://www.tilengine.org
Mozilla Public License 2.0
847 stars 95 forks source link

Fix minor warnings and some other errors. #40

Closed ghost closed 5 years ago

ghost commented 5 years ago

Muchos commits :smile: La idea es usar mucho el cherry-pick y aplicar sólo los cambios que estés de cuerdo.

El que necesita mas atención es: https://github.com/megamarc/Tilengine/commit/8979b251fd36b6e0cffba72df36de92226ea6f66 y todos los commits con descripción Fix sign: use ssize_t.. Explicación: LoadFile cuando genera un error indica -1, pero el parámetro es size_t que es unsigned. Lo ideal es usar ssize_t pero no se si es soportado por el compilador de MSC. Igualmente implemente una solución que encontré en la web, pero yo no tengo acceso a dicho compilador MSC, falta testear eso:https://github.com/megamarc/Tilengine/pull/40/commits/1f1e0344c8d45614fc7455a3bd799ce1c6a54b7c

megamarc commented 5 years ago

Gracias por tus aportaciones! Tengo que probarlo en Visual Studio para comprobar que no rompa compatibilidad (supongo que tu trabajas más con gcc en Linux). Veo que los tests automáticos de Travis (para Linux) y Appveyor (para Windows) los pasan correctamente sin errores tu pull, así que imagino que estará bien.

megamarc commented 5 years ago

Daniel! He estado probando los cambios, sólo he visto una cosa que debería pulirse. La función snprintf() que has introducido en LoadFile.c sólo existe desde VS2015, en versiones anteriores no está definida. Habría que condicionarlo de alguna manera ya que si no el compilador falla. No sé cómo hacerlo, para aplicar un parche sobre otro parche que no está todavía confirmado, y que al sincronizarlo con el master se mantenga tu autoría. Por lo demás, fantástico

ghost commented 5 years ago

Hola Marc!

Me alegro que este casi todo bien :smiley: No tengo problemas con el tema de autorías. Como yo no utilizo Windows tal vez quieras ingresar esa modificación. O sino mandaría un commit con esto:

#if (_MSC_VER) && (_MSC_VER < 1900)
    sprintf (path, "%s/%s", localpath, filename);
#else
    snprintf (path, sizeof(path), "%s/%s", localpath, filename);
#endif

He mirado éste sitio https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=vs-2017 y la versión para VS2015 comienza en 1900 así que el condicional daría TRUE para versiones anteriores. También tengo que hacerlo de esa manera porque para GCC se cumple que (0 < 1900) así, ahora: #if (0) && (0 < 1900) funcionaría para GCC dando FALSE. Y para VS2015 seria: #if (1900) && (1900 < 1900) daría FALSE.

Si estas de acuerdo con esto y que lo haga yo mando commit. Saludos

ghost commented 5 years ago

Ahí encontré otros strncpy, perdón por mandar otro commit entre la conversación de otro tema :smile:

megamarc commented 5 years ago

Hola, cómo quedó el tema? Hiciste el commit para la compatibilidad con VS < 2015? Si lo haces, acepto el pull request con todas tus mejoras

ghost commented 5 years ago

Hola Marc

Ahí mande el commit. Perdón por no contestar antes, estuve de viaje. :smile: