apoplexy v3.0 RC1 released

Windows and Linux editor of PoP1 (for DOS and SNES) and PoP2 (for DOS).
Locked
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

apoplexy v3.0 RC1 released

Post by Norbert »

I've just released version 3.0 RC1 of apoplexy.

The GNU/Linux version is available here.
The Windows version can be downloaded here.

RC1 means release candidate 1.
Basically a public beta, to allow users to report possible bugs.
Later, on February 4th, version 3.0 will be released.

NOTE: Mods created with Pr1SnesLevEd can be opened with apoplexy, but lots of custom tiles will show up as question marks. If you want to use both editors, IMO it currently makes (more) sense to use Pr1SnesLevEd for polishing after using apoplexy for prototyping.

Changes since the last version:
+ The program can now also edit PoP1 for SNES levels!
* Minor bug fixes.
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: apoplexy v3.0 RC1 released

Post by Norbert »

I should (and want to) add that there's no way I could've ever created this without David's extensive documentation on the Princed Wiki and on this Princed Forum. I also gave him (and Kaslghnoon) an additional 'thank you'-line in the docs/README.txt file of the packages.

David, in apoplexy.c, the Compress() function contains two "Why?" statements. I think I can simply remove those lines?
If you have time and feel like it, could you verify that the entire Compress() function that starts at line 3612 is indeed correct?
Decompress() should be fine, because you explained the algorithm here and it's not as complicated.
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: apoplexy v3.0 RC1 released

Post by David »

Norbert wrote:David, in apoplexy.c, the Compress() function contains two "Why?" statements. I think I can simply remove those lines?
1.

Code: Select all

		arLengthX[4] = 0; /*** Why? ***/
I think this is needed.
arLengthX[4] is used a few lines later:

Code: Select all

			if ((iLength > arLengthX[4]) && ((iLength > 4) || (iCopy < 255)) && 
2.

Code: Select all

		iDataLength = 0; /*** Why? ***/
I think it's not needed.
In Pr1SnesLevEd, it's there only because I copied that part from a place where it's needed.

Some other things:

Code: Select all

				iDataLength+=128;
				sToAdd[0] = (unsigned char)iDataLength;
				iSize+=AddCharByChar (sCompressed, iSize, sToAdd, 1);
				iDataLength-=128;
Feel free to remove the += and -= lines, and use sToAdd[0] = (unsigned char)(iDataLength+128); instead.

Code: Select all

	/* Pr1SnesLevEd and apoplexy differ from the original game. For example,
	 * 01 01 01 00 00 00 is compressed to 01 03 01 01 03 00 in the original
	 * game, but Pr1SnesLevEd and apoplexy use 86 01 01 01 00 00 00.
	 */
Maybe change 3 to 2 in if (iLength > 3) ?

But then "22 33 44 01 01 01 00 00 00 55 66 77" (12 bytes)
will become "83 22 33 44 01 03 01 01 03 00 83 55 66 77" (14 bytes)
and not "8C 22 33 44 01 01 01 00 00 00 55 66 77" (13 bytes)

This can probably stay as it is.

EDIT:
This:

Code: Select all

((iLength > 4) || (iCopy < 255)) && (iLength > 3)
should be:

Code: Select all

((iLength > 4) || ((iCopy < 255) && (iLength > 3)))
corresponding to (length>4 || copy_pos-start_pos<0xFF && length>3) in Pr1SnesLevEd.

But...
((A || B) && C) differs from (A || (B && C)) only if A=true and C=false.
But that is not possible in this case: (iLength > 4) && !(iLength > 3) is impossible.
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: apoplexy v3.0 RC1 released

Post by Norbert »

Thanks for taking a look at this.
David wrote:I think this is needed.
arLengthX[4] is used a few lines later: [...]
But the while loop already starts by resetting all X lengths to 0, and arLengthX[4] is not being (used or) modified before the "Why?" line...
David wrote:[...] copied that part from a place where it's needed.
Okay, I'll remove it then.
David wrote:[...] iDataLength+128
That is indeed a prettier solution.
David wrote:This:

Code: Select all

((iLength > 4) || (iCopy < 255)) && (iLength > 3)
should be:

Code: Select all

((iLength > 4) || ((iCopy < 255) && (iLength > 3)))
corresponding to (length>4 || copy_pos-start_pos<0xFF && length>3) in Pr1SnesLevEd.
Thanks for pointing this out to me, I did indeed make a mistake there. :)

PS: I think that particular line of Pr1SnesLevEd code in its current state is confusing. Brackets could be used to "reinforce the default order to avoid confusion". Even if you yourself are smart enough to understand the order, with more brackets a) you may be able to read your own code more quickly in the (faraway) future, and b) other people, who may not be as smart as you, may be able to read your open source code more easily. To prevent misunderstanding of the operator precedence "programmers often make liberal use of parentheses". I hope (it's clear that) I'm not being pedantic. I'm not trying to show off how smart I am when it comes to operator precedence. On the contrary, it's because I, like most people, are not as smart as you that I can explain to you when, where and why your code might become less readable. Not that I think you think I am, but I want to add that I'm not blaming you for any mistakes that I've made myself after studying your code. On the contrary, I'm grateful and happy that your code is open source and, as I wrote before, there's no way ever I would've been able to load/save anything without it.
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: apoplexy v3.0 RC1 released

Post by David »

Norbert wrote: But the while loop already starts by resetting all X lengths to 0, and arLengthX[4] is not being (used or) modified before the "Why?" line...
Umm, you're right. I did not look in both directions.
Norbert wrote: Brackets could be used to "reinforce the default order to avoid confusion".
Thanks, I added it now.
I also enabled all warnings, and the compiler found a couple of similar cases...
Locked