Bit set function not working correctly?
Good day,
I don't know how to ask this as I don't totally understand things but I'm having an issue.
My program reads tiles and tile sets from a game. They are 4bits per pixel. And are stored according to bitplanes.
This is working correctly but I came across a sprite where the top row of an 8x8 tile does not read correctly. Not sure why as other sprites are reading/writing correctly. If its the game or my code I do not know. Below is the code where the tile does not read correctly.
I think it might be a problem with the bit set code. Maybe there is a bit that is set different in the game???
ps. Sorry for all the questions! :(
I don't know how to ask this as I don't totally understand things but I'm having an issue.
My program reads tiles and tile sets from a game. They are 4bits per pixel. And are stored according to bitplanes.
This is working correctly but I came across a sprite where the top row of an 8x8 tile does not read correctly. Not sure why as other sprites are reading/writing correctly. If its the game or my code I do not know. Below is the code where the tile does not read correctly.
I think it might be a problem with the bit set code. Maybe there is a bit that is set different in the game???
ps. Sorry for all the questions! :(
|
|
|
|
Last edited on
The for loop for (int p = 0, t = 0; t < 5; t++) doesn't make sense.
The looping variable t is never used and nothing else changes between the iterations, so the code blocks within the loop do exactly the same thing over and over.
Also, when you find yourself numbering variables (Bitplane0, etc), it's probably time for an array.
It seems that the following would do the same as your code.
And in this loop, the j is pointless. Just use N.
Instead of this
maybe do this:
Also, what you are calling "white" in your color table has the rgb values for black, but since it's alpha is 0 it's actually just transparent. Do you mean it to be transparent?
EDIT:
I just noticed that in the code you've posted you never set the variable X.
Also, instead of using bitset, you can just do this:
The looping variable t is never used and nothing else changes between the iterations, so the code blocks within the loop do exactly the same thing over and over.
Also, when you find yourself numbering variables (Bitplane0, etc), it's probably time for an array.
It seems that the following would do the same as your code.
|
|
And in this loop, the j is pointless. Just use N.
|
|
Instead of this
|
|
maybe do this:
|
|
Also, what you are calling "white" in your color table has the rgb values for black, but since it's alpha is 0 it's actually just transparent. Do you mean it to be transparent?
EDIT:
I just noticed that in the code you've posted you never set the variable X.
Also, instead of using bitset, you can just do this:
|
|
Last edited on
| #include <atlstr.h> |
Are you actually using the Visual Studio Active Template Library component? The ATL's used to simplify using COM objects and creating ActiveX components. Windows type stuff that really isn't C++ stdlib related.
Trying to read/write image files by using ATL is a bit of overkill even if the library is used correctly.
be careful reading off the desktop as well. I have had permissions troubles on that, though it depends on how your machine is configured. If it works, fine, but be sure you are fully checking that the file opened successfully in case of any surprises.
if you have additional troubles after the above fixes, consider writing the image file back out in a new file to be sure you read it in correctly, as a tmp.png or something that you can open in an image editor. This basically proves that you read it in properly and got the one you wanted...
if you have additional troubles after the above fixes, consider writing the image file back out in a new file to be sure you read it in correctly, as a tmp.png or something that you can open in an image editor. This basically proves that you read it in properly and got the one you wanted...
| Are you actually using the Visual Studio Active Template Library component? |
CString is used which is from atlstr.h. CString should be replaced with std::string.
|
|
This is just an array (4 elements) of an array (10000 elements). As the size is fixed at compile time, std::array can be used with smart pointers so that allocated memory is automatically released. Something like:
|
|
or could simply use vectors:
|
|
Why use CString and char* when std::string should be used?
.seekg() should use a specified direction constant rather than a number
https://en.cppreference.com/w/cpp/io/ios_base/seekdir
Also there is no checking that the file has been opened OK.
https://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique
https://en.cppreference.com/w/cpp/container/array
Last edited on
@seeplus, I know where CString is from, I was kinda hoping the OP would explain why they might be using it instead of <string>. :)
Along with a number of design choices that could be made more bullet-proof. C++ containers vs. C-style arrays and new for instance.
Along with a number of design choices that could be made more bullet-proof. C++ containers vs. C-style arrays and new for instance.
Also, don't include cpp files :+)
Try to use
Always try to declare and assign a value to a variable in 1 line of code. Wait until you have a sensible value, zero is not always a good value. This will avoid problems with variable
Try to use
const and constexpr more.using namespace std; will bite you in the ass one day, just put std:: before every std thing. Plenty written about this on internet.Always try to declare and assign a value to a variable in 1 line of code. Wait until you have a sensible value, zero is not always a good value. This will avoid problems with variable
X
| TheIdeasMan wrote: |
|---|
| Also, don't include cpp files :+) |
Ack! I missed that! Oooopsie!
I suspect so did everyone else until now.
Wanna bet removing any #included .cpp files will resolve a whole bunch of problems? :Þ
| Always try to declare and assign a value to a variable in 1 line of code. Wait until you have a sensible value, zero is not always a good value. This will avoid problems with variable X |
Another suggestion, declare a variable as close to first use as possible, instead of the C way of declaring all variables at the head of the function.
Not this:
|
|
Do this:
|
|
About
using namespace std;...It is not wrong, it is not bad to use, but as mentioned it will cause problems later when you start writing your own libraries and choose commonly used designations you might not know are part of the C++ stdlib.https://stackoverflow.com/questions/1452721/whats-the-problem-with-using-namespace-std#1452738
Yes, it means a couple of more key strokes are "saved" but after a while always typing std:: becomes second nature. It has become so ingrained into my programming headspace to NOT type std:: slows me down. Waaaaay slower.
On a side note....prefixing std:: makes it explicit what I'm using, something from the C++ stdlib. Enhancing readability never hurts.
In the long run saving a couple key strokes is not worth the effort IMO, my fingers prefix automatically.
There is another way to help with
Muscle memory can be quite hard to undo: For a good 30 years I was always in the habit of first typing both opening and closing braces (or any other that come in pairs), then going back to put something inside. Now editors will do that auto-magically, although some of them are smart enough to recognise that behaviour and not do 2 closing braces.
std:: , depending on one's IDE / editor : keyboard shortcuts (is that the correct term?). One can use whatever menomics they wish, but say that sveci means std::vector<int>, type sveci and it is replaced with std::vector<int>. That is a saving of 11 keystrokes, more if it is std::unordered_multimap, and even more still if one uses their own class type often. I guess one downside is having to remember all the shortcuts, but one could get used to that too, provided the naming is consistent and logical.Muscle memory can be quite hard to undo: For a good 30 years I was always in the habit of first typing both opening and closing braces (or any other that come in pairs), then going back to put something inside. Now editors will do that auto-magically, although some of them are smart enough to recognise that behaviour and not do 2 closing braces.
Thanks everyone for taking the time to give me advice.
I know my code is horrendous but I'm still learning although slowly. :(
My code is messy and had left over variables from testing and trial and error. My code needs revising as you guys mentioned putting things into arrays. All makes sense.
btw the program code above does not give any errors but the full program(as mentioned in my previous forum topic.) still does.
and I fixed the issue with the tiles not writing correctly. There was a slight difference in the bytes of the game file.
Thanks again!
I know my code is horrendous but I'm still learning although slowly. :(
My code is messy and had left over variables from testing and trial and error. My code needs revising as you guys mentioned putting things into arrays. All makes sense.
btw the program code above does not give any errors but the full program(as mentioned in my previous forum topic.) still does.
and I fixed the issue with the tiles not writing correctly. There was a slight difference in the bytes of the game file.
Thanks again!
| Cyclone wrote: |
|---|
| My code is messy and had left over variables from testing and trial and error. |
Try using a version control system such as git. There are GUI plugins which are easy to use. Put your whole project under git control, check out files, make small changes - even if it is just one function, compile and fix until it works nicely, commit changes with a comment on what you did. Then it is possible to revert to an earlier version. One can do branches.
When you say: " ... have left over variables from testing and trial and error. " Consider that it is easier to use the debugger to see what is happening with variables during program execution, rather than inventing new variables, then printing them to see what their value is. One doesn't have to spend time going back to get rid of those extra variables. One can notice straight away uninitialised variables like your variable
X - provided that you added it to the watchlist, because they will show garbage values.Btw,
X is a terrible name for a variable. If one uses good names for variables, functions and classes, reading the code should be like reading a story of what is going on. The variable
X should be of type std::size_t because it is an array subscript, and a sensible initial value for it is zero in this case, because you are writing to the start of the array. std::size_t is usually the largest unsigned type the system has, but this is not guaranteed by the c++ standard. On my system it is unsigned long which is 64 bit on Linux, it is probably 64 bit on your windows system too, but it will be unsigned long long . One should use std::size_t with all the STL containers, for example the size() functions return a type of std::size_t, one can avoid a compiler warning when there is a comparison between int and std::size_tAnother thing about "messy code": there is a rule about how much code should be in a function, some folks go for as little as 25 Lines Of Code (LOC), the bombshell is that it includes the main function !! Part of the reasoning is that a function should only do one conceptual thing only. Higher level functions can call multiple lower level functions to do their work. The other reason is that it makes it easier to manage the project. Once a small function has been proven to work, one does not need to worry about it any further, apart from what arguments were sent to it, but the function should validate them anyway.
About design: People seem very reluctant to write down / draw their design on paper. Consider writing into a file a series of comments which describe what one wants to do. One can identify what needs to be in a class, what code could be in a function - loops say, what code will be in different files etc. Then go back and write the code which does what was in the comment.
When coding, write a small amount - even if it is just 1 function and the call to it, compile and test to make sure it works before you continue. This way better than charging in and writing 100's of LOC, then wonder why there are lot's of errors.
This guide is written by Bjarne Stroustrup and Herb Sutter, you may find it useful to look as your coding progresses :
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md
Finally I do hope you are using an IDE? IMO it is much easier for beginners, because of things like intelli-sense which show problems as one types. Perhaps one could learn more by using a plain editor and the shell / command line to compile, but one really does learn the hard way, which can be frustrating.
Anyway Good Luck !!! :+)
Topic archived. No new replies allowed.