Add support for h2d file format with first data file #3828
Conversation
ihhub
commented
Jul 11, 2021
ihhub
commented
- fix farm rendering in Knight castle (Castle screen, fully built castle, differences noticed #2059)
- add missing farm sprite made by our artist (close Only half of a farm is drawn in Knight city without castle #1080 )
- fix farm rendering in Knight castle (#2059) - add missing farm sprite made by our artist
ihhub
added
improvement
|
Hi @oleg-derevenetz and @idshibanov , this is the first step for storing any data files for the upcoming add-on so could you please light out your opinion about the way how we can store files? |
Hi @ihhub Is there a reason to store them as a separate files? Why not store them in binary itself as a some sort of resources (for example, as trivial |
In the future we're going to add much more resources including new monsters or sounds for example. We're talking about megabytes of data. I don't think that keeping them in binary would be the best solution. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| for ( const auto & data : _fileData ) { | ||
| fileStream.putRaw( reinterpret_cast<const char *>( data.second.data() ), data.second.size() ); | ||
| } |
There was a problem hiding this comment.
warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]
fileStream.putRaw( reinterpret_cast<const char *>( data.second.data() ), data.second.size() );
^| image.resize( width, height ); | ||
| memcpy( image.image(), data.data() + 4 + 4 + 4 + 4, size ); | ||
| memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size ); | ||
|
|
There was a problem hiding this comment.
warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]
memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size );
^| image.resize( width, height ); | ||
| memcpy( image.image(), data.data() + 4 + 4 + 4 + 4, size ); | ||
| memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size ); | ||
|
|
There was a problem hiding this comment.
warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]
memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size );
^| image.resize( width, height ); | ||
| memcpy( image.image(), data.data() + 4 + 4 + 4 + 4, size ); | ||
| memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size ); | ||
|
|
There was a problem hiding this comment.
warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]
memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size );
^| image.resize( width, height ); | ||
| memcpy( image.image(), data.data() + 4 + 4 + 4 + 4, size ); | ||
| memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size ); | ||
|
|
There was a problem hiding this comment.
warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]
memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size );
^| image.resize( width, height ); | ||
| memcpy( image.image(), data.data() + 4 + 4 + 4 + 4, size ); | ||
| memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size ); | ||
|
|
There was a problem hiding this comment.
warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]
memcpy( image.transform(), data.data() + 4 + 4 + 4 + 4 + size, size );
^
|
Hi @oleg-derevenetz , looks like we have to disable |
Yes, I agree regarding the first two checks, but regarding the |
Sure. Let's do this way. |
| const int32_t height = static_cast<int32_t>( stream.getLE32() ); | ||
| const int32_t x = static_cast<int32_t>( stream.getLE32() ); | ||
| const int32_t y = static_cast<int32_t>( stream.getLE32() ); | ||
| if ( static_cast<size_t>( width * height * 2 + 4 + 4 + 4 + 4 ) != data.size() ) { |
There was a problem hiding this comment.
either cast from 'int' to 'size_t' (aka 'unsigned long') is ineffective, or there is loss of precision before the conversion
| return false; | ||
| } | ||
|
|
||
| const size_t size = static_cast<size_t>( width * height ); |
There was a problem hiding this comment.
either cast from 'int' to 'size_t' (aka 'unsigned long') is ineffective, or there is loss of precision before the conversion
|
Hi @oleg-derevenetz , I addressed your comment. Could you please take a look? Right now it's not obvious why we need h2d file but in the future with add-on development (which started not long time ago) we have to ship all new assets altogether. To reduce user errors such missing files during copying. |
|
Hi @Branikolog , could you please also test these changes? Just install a fresh version of the game and check a town with the farm in Knight castle. |
|
Hi, @ihhub |
| } | ||
|
|
||
| for ( const auto & data : _fileData ) { | ||
| fileStream.putRaw( reinterpret_cast<const char *>( data.second.data() ), data.second.size() ); |
There was a problem hiding this comment.
do not use reinterpret_cast
| const int32_t height = static_cast<int32_t>( stream.getLE32() ); | ||
| const int32_t x = static_cast<int32_t>( stream.getLE32() ); | ||
| const int32_t y = static_cast<int32_t>( stream.getLE32() ); | ||
| if ( static_cast<size_t>( width * height * 2 + 4 + 4 + 4 + 4 ) != data.size() ) { |
There was a problem hiding this comment.
either cast from int to size_t (aka unsigned long) is ineffective, or there is loss of precision before the conversion
| return false; | ||
| } | ||
|
|
||
| const size_t size = static_cast<size_t>( width * height ); |
There was a problem hiding this comment.
either cast from int to size_t (aka unsigned long) is ineffective, or there is loss of precision before the conversion
| @@ -289,7 +289,7 @@ namespace CastleDialog | |||
| } | |||
|
|
|||
| private: | |||
| uint32_t _alpha; | |||
| uint8_t _alpha; | |||
There was a problem hiding this comment.
use default member initializer for _alpha
| uint8_t _alpha; | |
| uint8_t _alpha{ 255 }; |
|
SonarCloud Quality Gate failed. |
