Omitting brackets is considered a bad practice by many, though, as you sacrifice maintainability (I used to omit, but have since changed my ways). And having multiple exit points can be considered a bad practice (especially noticeable when debugging multi-threaded applications), but I personally never worry about that unless I have to.

I'm sure it's probably safe in the code you changed, but your replacement is not logically identical to the original code.

Oh darn, was just about to release a massive overhaul to the project (all the files are now source/header pairs, fixed a number of memory leaks/poorly written deletes), I'll look very closely at your edited code for your changes and will bring over all of them that I can, thank you very much!

Curious, are static functions advantageous or for book-keeping? answered in sb

Edit:
I copied over all the changes I could find and removed the stringy file; attached is where it's at atm (will post an 'update' after headers have been commented and undo/redo is finished).

Thanks for introducing me to sprintf, seen it around but didn't understand it till seeing it replace my code

Attachments:

Post has been edited 3 time(s), last time on Jan 28 2013, 8:06 am by jjf28.

How so, exactly? Replace "condition" with either true or false and give me a scenario where returning it yields a different result. Let's break it down:


Because the condition must evaluate to a boolean, those two snippets of code are the same, correct? So only if the condition is true will it return true, and only if the condition is false will it return false. Therefore, you can just return the condition as Heinermann was suggesting:

That's why a naming convention for boolean variables is to start it with "is" or "has." For example, consider the variable being called "isAllowed" in the above scenarios.

Static functions are fine for arithmetic/stateless operations. Otherwise, you should try to avoid making your functions static.

Fell into the same trap as Heinermann, Roy

Assuming true = 1 and false = 0, here are the input/outputs for each case:
if (condition) return true, else return false:
input 2 return 1
input 1 return 1
input 0 return 0
input -1 return 1

return condition:
input 2 return 2
input 1 return 1
input 0 return 0
input -1 return -1

Also from a defensive programming perspective, I'd say it's much better to use the former, because it guarantees the function will only return 0 or 1 regardless of the inputs. The latter code will probably also return 0 or 1 because of the structure of your code, but there's always a chance that some random input could show up, causing it to return a very unexpected value, which could cause problems elsewhere in your program.

The input cannot be 2 or -1, because the code inside an if statement must return a boolean value to be evaluated. Therefore, the condition variable can only be 0 or 1. C is a staticly-typed language: when you define a variable, it will be that type. If your function returns a boolean and you try to give it a 2 or -1 (or an int variable in general) to return, it will be a compile error.

Unless I fundamentally do not understand some quirk of C/C++.

Post has been edited 1 time(s), last time on Jan 28 2013, 6:27 am by Roy. Reason: Boolean types don't exist? :(

C doesn't have a boolean type. Hence why I said "assuming true = 1 and false = 0".

That's why most things say it like "returns nonzero on success/failure/what". In C, true is just "nonzero" and false is "zero", so ultimately it doesn't seem like it should matter unless you give some importance to the actual binary value.

And when you use the return of that function in if(retval) you get ...
retval 2 return 1
retval 1 return 1
retval 0 return 0
retval -1 return 1
So, really, it shouldn't matter too much especially when, no matter how you choose the return value, the return value is going to have two cases: nonzero and zero.

EDIT:
Unless you are using if(condition == true) ... but who does that?

EDIT2:
Unless we are just assuming "condition" is a single variable, and not a placeholder for like "thing >= value".

Post has been edited 4 time(s), last time on Jan 28 2013, 8:19 am by FaRTy1billion.

Unless you do retvel = function(args), in which case returning something other than 0 or 1 could potentially cause problems down the line if you aren't careful.

My whole point though is that it's potentially dangerous, not that it is dangerous.

I disagree. As for having multiple exit points, I think it's fine to immediately return after checking validity of a call (a piece of code that is unlikely to change). But ultimately this is jjf28's decision, I just wanted to show that inverse statements can be used. I think jjf28 should consider your post though.

We're assuming 1 = 1 and false = false. This is C++. But even in C it wouldn't necessarily be bad. You would define FALSE as 0 and just check != FALSE (or just don't compare it with anything). But in C++ returning a condition as a boolean, even if your condition is not a boolean expression, will implicitly cast it to bool (using 0 = false, otherwise true).

I didn't say it was bad, I said it was unsafe and not logically the same as the original code, which is true. My assumption was C however, not C++, so fair cop on that.

Here are a few other changes that can be done:

to




may be valid, but in C++ you are supposed to use

Just drop the .h and add a c in front (doesn't apply to windows.h, just the C standard library).

Some code for your Math.cpp:

(EDIT: Not sure about the above. If it's no good then in your code use bitwise shift operators instead of dividing by large constants)


Also want to introduce you to the C++ standard library (slowly)


Eventually your Clipboard/Selection stuff will use the C++ Standard Template Library (STL) containers to hold information so you don't have to manually manage their pointers (right now you're emulating what you could do using std::forward_list<TileNode> and std::forward_list<PASTETILENODE> ). Using the standard library will help minimize the risk of errors. If you're not ready for the upgrade then you can leave it as is for now.

In Tilesets.cpp you used a bunch of strcat calls to initialize OpenFilter, but you can add null characters using sprintf with the escape sequence \0.


Lastly, you have a hardcoded stardat.mpq location. BWAPI has some code for retrieving the Starcraft install directory here: https://code.google.com/p/bwapi/source/browse/branches/bwapi4/bwapi/BWAPI_PluginInjector/common.cpp#20

Look at GetSingleRegString and GetRegString, both of which use mostly C-style code for the WinAPI calls.

Post has been edited 4 time(s), last time on Jan 30 2013, 4:21 am by Heinermann.

"Could not find StarDat.mpq

Would you like to locate it manually?"

Clicking yes just closes the program, but it remains as a process within task manager.

I was getting a rather curious bug where compiling in debug rather than release mode caused openfilename calls to fail, should be fixed in the next release

Edit: Found the specific discrepancy; apparently the compiler didn't like me messing with heap size

Post has been edited 1 time(s), last time on Jan 31 2013, 5:43 pm by jjf28.

Update 3/20/13 - Download Latest

- Basic unit and sprite display (without player-color or ISCRIPT based modifications)
- Unit and unit sprites displayed on minimap (1 pixel per)
- Terrain select-all
- Terrain copy-paste
- View-only mode for protected maps (no saving/copying)
- Protected map detection: duplicate sections, oversized sections, invalid VCOD
- Supports open-with and drag-and-drop
- Automatically gets starcraft directory, otherwise better handling
- No longer deletes or rearranges chk sections
- No longer limits how many maps can be open at once
- SFmpq.dll no longer required with the program
- Fixed misc creation and saving errors (including those due to out-of-storage situations)
- Misc crash and bug fixes



If anyone's following the code there's been a fair amount of restructuring, perhaps most notably, maps now contain a linked list of chk sections and pointers to the sections Chkdraft uses (or plans to use) - these pointers can be used safely as each method in the buffer class validates that they are not null.

Query: is the main Starcraft unit pallet hard-coded? (can't find it anywhere...) I've hard-coded one provided with starforge for now - seems to give good results.

Text triggers was ~85% complete when I ran into strings, put that off till later.

All in all there aren't that many noticeable 'functional' changes with this update, regardless I hope you enjoy!

Post has been edited 1 time(s), last time on Mar 20 2013, 12:16 pm by jjf28.

Good job so far! It looks like it's coming along really nicely.

There is no specific unit palette. Use the WPE from the active terrain files as the global palette (that's what StarCraft does).

Also, if you don't already know, game\tunit.pcx has the player colors. It's a table with 16 sets of 8 (only the first 12 are used, referenced by COLR section in BW maps), and the 8 colors are mapped to indexes 8 through 15 when drawing unit sprites (if, for example, the grp wants to draw color index 9, then read the color index at tunit[pcolor*8+(9-8)])

Post has been edited 4 time(s), last time on Mar 21 2013, 1:40 am by FaRTy1billion.

Simple enough, thanks ¹


I copied the colors seen from a minimap pic, this will be better


See #1

This seems dangerous, since Chkdraft is open-source now. Let's #define noProtection 1
+ Does this mark map which is missing editor-only sections as protected? (Such as IVE2, WAV, ...)

I removed that flag and the VCOD failure position echo, and re-uploaded.

With the project being open source and the code cleanly written there's no real way for me to support map protection against programmers unless I purposefully write my map parser worse (no).

Ultimately unprotection tools means zilch starcraft programmers as they can unprotect the maps themselves (or can easily find means to do so). In any case, as i'm not releasing a utility that can modify protected maps, or releasing instructions to modify chkdraft to disable protection, or purposefully providing a means to do so, SEN's unprotector rules don't apply here


Edit: No it does not consider maps missing (a) section(s) to be protected

Post has been edited 1 time(s), last time on Mar 21 2013, 4:25 am by jjf28.

I think it's really, really cool you're doing this.