Frederic Cordier notified Power2People that he considers the AmiDark bounty project to be fulfilled.

The source code of the AmiDark Engine has been made available at:
https://sourceforge.net/projects/amidarkengine/

You can review the promised feature set by reading Power2People´s project requirements:
http://power2people.org/projects/amidark/

Power2People relies on the Amiga, AROS and MorphOS communities to ensure that a bounty project has been successfully completed. So, please review the published source code and post if you have encountered any problems or know other reasons why the bounty project cannot be considered to be completed just yet.

Also, if anybody would like to reward the development work before the project is closed but has yet to do so, now would be the time to contribute.

Thank you very much.

UPDATE: Please post newly discovered bugs in the following discussion thread instead:
http://amigaworld.net/modules/newbb/viewtopic.php?topic_id=39807&forum=15&0

Last edited by ASiegel on 16-Jan-2015 at 09:46 AM.

@ASiegel
I've posted a link to there on amigans.net :
http://www.amigans.net/modules/xforum/viewtopic.php?forum=25&topic_id=6799&order=

@AmiDARK
How strict do you want me to be when reviewing?

_________________
AmigaOS 4.1 FE (sam460ex Radeon 9200 / RadeonHD), MorphOS 3.8 (PowerMac G4 733MHz Radeon 9000), AROS (x86), A1200 (060 80MHz Indivision MK2), A500, A600, CDTV
Wings Remastered Development Diary

@Daytona675x
The object of the review is to tell to Power2People if the source I've released correspond to the Bounty request.

@AmiDARK
So I asume being stable and reasonably bug-free is important.
In its current form I'd say that this lib cannot be used reliably / doesn't deliver what's promised in a stable (or even efficient) way.
I didn't check everything yet and I did not look in depth, but I really found enough already to come to this verdict.

The whole lib contains lots of array-bounds-errors, potential division by zeros, documentation inconsistencies, etc.
For example try Memblock ID 256.
Or: DECreateBitmap wants an ID>0 and < 8. But the docs say >=0 and < 8.
Or: DELoadBitmap accepts an ID>0 and < 9 (nine!). But the docs say >=0 and < 8. And the internal array will blow when you come with an index 8...

Such things are there all over the source (and no defines nor consts, always plain numbers).
Actually even some basic functions like for example INTERNAL_CalculateNormal don't do what they should do

However, the biggest show-killers I found so far are the Text functions.
Many of those have internal silent and undocumented string size limits of 256 bytes.
They simply don't do what you expect them to do or what the documentation says they do.
For example DELeft. It will at max. return a string of 255 chars (+ 0 byte), even if you expect it / instruct it to return more.
Other functions like DELower (which contains other fun as well) will just blow if you feed them with a string > 256 bytes.

Then I found many extremely (!!) unefficient sections. One nice example for this is the joystick code. Here every check like Up/Down/Button1 issues a seperate ReadJoyPort call.
While things like that do work without crash I'd consider them as no-goes in terms of efficiency. You cannot put code like that in something you call a game engine IMHO.

The same goes to the use of OpenGL's immediate mode. For tutorial-code purposes this may be okay, but it has absolutely no right to exist inside a game engine's code-base.

Sorry if all that sounds a bit harsh, but since people payed for that source I can't be too tolerant.

At least all those crash-bugs have to be fixed until this can be of any use to anybody. I guess the efficiency-issues can probably be considered "acceptable for now".
Since the lib isn't that big it should be possible for you to fix those issues rather quickly.

And please add example code to the help!
I was pretty shocked when I saw that the help's example section was empty.
I mean: the idea was to have a tool noobs can use, right? But in it's current form only professionals have the slightest chance to workaround all those issues.

_________________
AmigaOS 4.1 FE (sam460ex Radeon 9200 / RadeonHD), MorphOS 3.8 (PowerMac G4 733MHz Radeon 9000), AROS (x86), A1200 (060 80MHz Indivision MK2), A500, A600, CDTV
Wings Remastered Development Diary

@Daytona675x
In the bounty, it is said that the engine is not yet finished ! But, as demonstration available in os4depot, the available functions works !

You should really see how DarkBASIC Professional & DarkGDK works as The AmiDARK Engine is entirely based on their function set to allow compatibility with these products.

Concerning what you said :

You'll see that Bitmap 0 = Current Display output.
Ok for LoadBitmap, changed for 8, will update when all informations will be available.
Concerning other array-bounds-errors ... Where ?
potential divisions by zeros, where ?
Documentation is NOT finished as the engine is not finished. It was said for the bounty.

INTERNAL_CalculateNormal is a function that directly came from DarkBASIC Professional source code I've written for a plugin TheGameCreators resell and it calculate 3D normals correctly. You're wrong concerning this statement.

Plain numbers are used where things must not be changed, consts and defines are used when needed.

The text functions works like they are supposed to, The Technical demonstrations available at os4depot in the latest release show this. Then can you be exactly precise concerning the functions you are asking that are "wrong" ?
More to this, the 256 bytes limitation of text is also a COMPATIBILITY with DarkBASIC Professional/ DarkGDK products. They work the same way. I wanted to keep AmiDARK Engine compatible... That's why you see DELeft and DELower being limited at 256 bytes.
As it is not yet finished, documentation is not yet finished ... all was previously mentioned "product unfinished", "doc unfinished".
Don't forget that there are also "internal function" that are not user functions and that are only for the engine needs ... Take care to not make errors when looking in a function...

Concerning Joystick, it's the only command set I didn't test and I'm not sure
And concerning the fact that buttons are checked separately, don't forget that it's a game engine and developper will maybe not have to use 2nd button or 3rd... that's why things are checked separately .. It's also the same commands names and functions than in DarkBASIC Professional / DarkGDK and they are professionnal products ...
OpenGL Immediate mode are only for 2D drawing stuffs ... It's extremely rare that we use them in a game engine ... 3D Models, Images, for example don't use immediate mode. As the objective of that kind of engine is the flexibility, there are things that canot be developed the same way a game will be.

Concerning Memblock, DarkBASIC Professional allow 1 to 256 that's why it's the same in the AmiDARK Engine.

All those crash-bug ? Which ones ? The Joystick functions ? because others works correctly as you can see in the latest release at os4depot.net or the binaries available in the opensource project.

The HELP is also based on the DarkBASIC Professional help. It will be like in DarkBASIC Professional..

You were shocked ? Then in my turn, I am shocked that you didn't read carefully the bounty that mentionned clearly at this point :
More information concerning the current state of its development can be found directly on the official AmiDark website.. With the link pointing there : http://www.amidark-engine.com/spip.php?article38
Showing the development state of the engine ...
Directly in the bounty :
The engine contains a PARTIAL HTML documentation explaining how the commands / functions work (632 commands / functions done)

your review apparently started from the idea that the engine was completed and it is clearly mentioned that it was not ... Don't forget that this kind of game engine, if it was finished may have it's price really higher than 1500€ ... Maybe something like 20.000€ or more ...

Regards,
AmiDARK

Last edited by AmiDARK on 10-Jan-2015 at 09:21 PM.

@AmiDARK
Calm down. All what I said remains true, all those bugs are there.
And whatever the bounty goals tell:
I'm pretty sure people want something that does NOT crash all the time and expect that the functions you marked as "green" are working. Well, sorry, some of them just don't.
Don't point at me - you are the one who wrote it

It's not really my homework to do, but since I'm in a good mood I'll help you although I dislike your attitude. Deliver what you're paid for and don't get pissed if somebody finds out that what you sell doesn't work.

Quote:

You'll see that Bitmap 0 = Current Display output.

Documentation?

Quote:

potential divisions by zeros, where ?

VECT_Maths3D.c, line 45 and line 56
for example.

Quote:

INTERNAL_CalculateNormal is a function that directly came from DarkBASIC Professional source code I've written for a plugin TheGameCreators resell and it calculate 3D normals correctly. You're wrong concerning this statement.

No, sorry man. Just because you copied it from somewhere else doesn't make it correct...
B3D_FVF338.c, line 105 is plain bullshit. It must be a >0 check, not >1...
If you want to write a game-engine you should at least know some of your math...

Quote:

The text functions works like they are supposed to, The Technical demonstrations available at os4depot in the latest release show this. Then can you be exactly precise concerning the functions you are asking that are "wrong" ?

Just take a look at TXT_Functions.c, lines 166-180, the function DELower. And now tell me what happens if you call that function with a char* pointing to lets say a 1000 character string.
Do you really want to tell me that this function will behave as it is supposed to be?
Besides the potential crash here (a situation that happens because on the one hand you limit the out-buffer to 256 but your len-variable may well be much greater): if you have a 256 characters limit by design in those functions then the docs should say so.

Quote:

Concerning other array-bounds-errors ... Where ?

Everywhere. We just got one: line 174 above And prepare for another one...:

Quote:

Concerning Memblock, DarkBASIC Professional allow 1 to 256 that's why it's the same in the AmiDARK Engine.

Yeah, all fine. But the global arrays you define in Memblock_Variables.c only allow for indices between 0..255. But you allow accessing them with 256 (all your funny < 257 checks all over the place allow that invalid index to be propagated)... A classic array-bounds-crash-bug.

Quote:

Plain numbers are used where things must not be changed, consts and defines are used when needed.

What I mean is that instead of things like
struct MusicStruct Music[ 16 ];
(which is the style you use all over the place, for example also above in the Memblocks mess) it would have been much smarter to use a const / define. Would also helped against some array-bounds bugs.

Quote:

Don't forget that there are also "internal function" that are not user functions and that are only for the engine needs ... Take care to not make errors when looking in a function...

... ehm... a severe bug in an "internal" function is just as severe as somewhere else. I don't see any reasonable reason why I should not check your "internal" functions for bugs?! Doesn't make sense.

Quote:

And concerning the fact that buttons are checked separately, don't forget that it's a game engine and developper will maybe not have to use 2nd button or 3rd... that's why things are checked separately ..

You don't get it. A decent engine would take care that the costy ReadState calls are minimized. That call should be done once per frame and the respective functions should operate on a cached int. Or at least provide a function GetJoystickState or so so people can optimize it themselfs.
But as being said: such inefficiencies are probably really out of the bounty's scope.

Quote:

your review apparently started from the idea that the engine was completed and it is clearly mentioned that it was not ...

No. As I mentioned above to make it clear:
what I expect is that what you marked as "green" is working flawlessly.
As I already proved this is not the case.

Quote:

Don't forget that this kind of game engine, if it was finished may have it's price really higher than 1500€ ... Maybe something like 20.000€ or more ...

Come on, let's stay serious. You're WAY overestimating the value of that code's quality / it's feature-set. If you fix the bugs it's worth what you got, no less no more.

I'll extend the bug-list from time to time.
Cheerio!

Last edited by Daytona675x on 10-Jan-2015 at 10:31 PM.
Last edited by Daytona675x on 10-Jan-2015 at 10:02 PM.
Last edited by Daytona675x on 10-Jan-2015 at 09:46 PM.

_________________
AmigaOS 4.1 FE (sam460ex Radeon 9200 / RadeonHD), MorphOS 3.8 (PowerMac G4 733MHz Radeon 9000), AROS (x86), A1200 (060 80MHz Indivision MK2), A500, A600, CDTV
Wings Remastered Development Diary

@Daytona675x

Not actually understanding how serious or not the points you are bringing up are, and I also dont think i ever read the bounty text mor than quick looked it, since i was mostly reading from here what AmiDark wrote about it, and i have to point out that at least my impression was that it isnt very usable engine at this point yet, that there was still a lot of work to be done. And the whole point was that AmiDARK needed some money very badly, and hence he decided to make it open source so to say (not sure what is exactly the right term here), from which I even more expected that it wouldnt be in usual sellable condition yet.

Cant of course say what others thought, but so far sounds like it is in state i would have expected it to be based upon what he wrote here in amigaworld.net thread.

But once again I like to point out that I dont really understand if those things you mentioned are big things or not, since I dont code in that level, and hence I am not able to understand the importance of those things. That should he have got them right in the first place, or are those things that you just look in the end through your code and fix them just like that. Or perhaps they are even very difficult to fix?

But I anyway want to also thank you for poiting those things out, both to help those who supported the bounty evaluate if they think they are happy about it, as well as helping AmiDARK find problems from code and fix them, since this way we all may benefit in future from your comments.

Last edited by Bugala on 10-Jan-2015 at 10:03 PM.

For me, the bounty was to open-source the AmiDark code AND that Frédéric continues to work on it.

The first part is completed, his GDK is open sourced (I don't have the knowledge to say if the code is good or not).

AmiDark, will you continue to work on it ?

Daytona, thank you to spot the problem on his code/tutorials, it will be certainly useful for Frédéric.

Frédéric, it's up to you

_________________
http://apps.amistore.net/zTools

@Daytona675x

Once again your DON'T READ CAREFULLY :

Quote:

Documentation?

Not yet available .. Dev status : Unfinished , mentioned in bounty.
I really have the feeling that you don't read the "rules" of the bounty and the project informations available.

Quote:

VECT_Maths3D.c, line 45 and line 56 for example.

I understand that here you're speaking about DIST that can be 0... But ... in floating single precision .. Perfect 0.00000000000000000 never exist ... Not ?

Concerning the DELower, it is planed to add to the doc the fact that strings must be created/Deleted using the LCreateString/LDeleteString or LCreateDeleteString function to keep compatibility with DarkBASIC GDK working way and DarkBASIC Professional internal TPC development working way. That mean in the final doc user will know he can use only 256 bytes strings. (DarkBASIC GDK that is the C/C++ version of DarkBASIC Professional uses CreateDeleteString function for all strings purposes ... Compatibility with it)... Not yet available ... For the same reasons explained many times here and before ...

Line 174 : OutPut[ sLoop ] = tolower( OutPut[ sLoop ] ); ?
Same sentence than previous one.

Concerning memblocks ... Excuse me ... too fast to write ... Here is a copy of DBPro help concerning memblocks :
Quote:

MAKE MEMBLOCK Memblock Number, Size in Bytes emblock Number Integer The memblock number within the range of 1 to 255 Size in Bytes Integer The size of the memblock in bytes

So I'm ok on that.

Concerning musics .. Better to use CONST ? No.. CONST are interesting if you plan to further change the limits .. But it the value is a definitive one. Why should I create a CONST ? It's only a point of view and not a true argument for good/bad coding.
So for Music, it's not a bug it is what it should.

And the engine is not unstable ... If you use functions "like it should" all shoudl be ok. Maybe some extra "checking" can be add to be sure that developer will not do "anything uncorrect" that may cause problem ... But, as the available demonstrations show, the engine run fine without crashing ! Contrary to what Daytona said.

Last edited by AmiDARK on 10-Jan-2015 at 10:27 PM.

@AmiDARK
Quote:

I understand that here you're speaking about DIST that can be 0... But ... in floating single precision .. Perfect 0.00000000000000000 never exist ... Not ?

... You obviously did not understand floating point math. The division by zero can happen easily.

Quote:

Concerning the DELower...keep compatibility with DarkBASIC GDK working way and DarkBASIC Professional internal TPC development working way. That mean in the final doc user will know he can use only 256 bytes strings.

Ah, so you're going to either fix the functions or fix the docu in the future?
But right now most of the text functions don't work as expected, right?!
Why are they marked green then?
See, that's my point: you claim that this lib is much more "green" than it actually is...

Quote:

Concerning memblocks ... Excuse me ... too fast to write ... Here is a copy of DBPro help concerning memblocks :... So I'm ok on that.

"Ok on that?" You mean you realized that your code is buggy? Or do you still think it is okay? If you do so I have to say it again: no, it is buggy. Your code allows invalid indices to be used -> crash. This remains true no matter how much you twist it.

Quote:

Concerning musics .. Better to use CONST ? No.. CONST are interesting if you plan to further change the limits .. But it the value is a definitive one. Why should I create a CONST ? It's only a point of view and not a true argument for good/bad coding.

Conerning musics? No, concerning your hardcoded array-limits in general.
No, it's not only a point of view. Most of the array-bounds-bugs (the ones you apparently still keep and continue to deny for whatever weird reason) would probably not have happened if you had used consts / defines.

Quote:

And the engine is not unstable

It is not stable. It contains severe bugs. And I only scratched the surface...
But now you made me kind of "hot"... Now I'll take a day off tomorrow and inspect it in depth

Quote:

If you use functions "like it should" all shoudl be ok.

"should" C'mon. I already pointed you to concrete functions and concrete lines that don't work even if used "like it should", at least in terms of the docs etc.

Quote:

But, as the available demonstrations show, the engine run fine without crashing ! Contrary to what Daytona said.

Don't quote me wrong!
I did NOT say that your demos don't work. Probably they work well. But that does not mean that the engine is working well. Just because a program works under circumstances X that does not mean that it also work under circumstances Y.
Your engine is most likely stable enough for your demos. But I can quickly come up with a demo that will crash - by only using your engine the way the docs tell me I should use it.

@Bugala
In my opinion this engine needs a quite some fixes until it the bounty can be accepted. At least the functions marked as green in the commands-set-state chart should really work. Apparently that's not the case.
If AmiDARK would concentrate on fixing instead of trying to defend or deny his bugs this could be done rather quickly

Last edited by Daytona675x on 10-Jan-2015 at 10:53 PM.

_________________
AmigaOS 4.1 FE (sam460ex Radeon 9200 / RadeonHD), MorphOS 3.8 (PowerMac G4 733MHz Radeon 9000), AROS (x86), A1200 (060 80MHz Indivision MK2), A500, A600, CDTV
Wings Remastered Development Diary

Updated 4 files with changes adviced at aros-exec.org + the Bitmap limit 9 to 8 change.

@AmiDARK

You evaded my question "AmiDark, will you continue to work on it ? "

Never mind if the code is good or not, maintainable or not, if the original coder is still in the boat.

_________________
http://apps.amistore.net/zTools

@Thread

I was one of the bigger donators and was fully aware of the fact that its "In Development".

My impression before I even donated that part of the reason for the "fundriser" was to keep AmiDark in a position where he could even justify developing for AOS4 considering his health and general financial situation.

Likewise my impression was that AmiDark was reaching out for more input to accelerate the development, which also entails bug reports and suggestions about how to improve aspects of the engine.
With that in mind I think Daytona675x's comments should be viewed as very CONSTRUCTIVE since hes being detailed and technical.

So bottom line;

I donated cause I like the purpose and direction of the AmiDark engine.
I did not expect a flawless engine, espesically since it on the fundriser page said it was to be developed futher.
I hope/expect AmiDark takes all input regarding his engine as constructive input, even if he disagrees with it. If nothing else; it leads to a public discussion that raises the awareness of the community of the Engine. Hell, we might even learn a thing or two following your exchanges

Happy to see the bounty reaching over 100%, and Happy New year to you AmiDark

Last edited by Overflow on 10-Jan-2015 at 10:56 PM.

@ZZD10H : As mentioned in the bounty :
If this project is successfully funded (by raising a minimum of 1500 Euro), and as long as the resulting ports stay true to the original version of the AmiDARK Engine in terms of user experience and command sets, Frédéric Cordier plans to share all source code that he will develop himself for future versions of the AmiDARK Engine for free.
So it should be :p
But for real-time testing, I'll have to find an AmigaOS4, MorphOS and AROS computer.

Overflow : I understant that some information Daytona675x said are important for a "final product" .. but I want to point on the fact that these problems does not prevent the engine from running fine (at its actual development state alpha/beta ... if developer uses the command in the range they are developed for) without crashing.
And, some more checking can be added to prevent developer error when using a command/function ... But the main objective of the development (in the past) was firstly to get the maximum amount of commands/functions to run. After this, the debug/improvment/checkings can be added to add more stability to the engine.
These are things that are planed for futures updates. And it's normal to be perfect for a "final product" but we are not in this case for now!
So, at a final statement. it's not the information he give that are not good (in my point of view), it's the way he do that.

@AmiDARK



Well, forums are bad at conveying "tone and attitude" when reading/making posts; so I generally just extract the useful information I get from a post, since I realise that I can easily misunderstand the "mood" of the poster.

Im my honest opinion it does neither you nor Daytona675x any good to get "worked up" over the "tone" of a post.

Both of your are skilled coders/developers: so its better for everyone you exchange information instead of focusing on "emotions".
Ive had many forum exchanges over the years, and usually people get worked up over misunderstandings. Usually 1 minute on voice communication like Teamspeak, mumble etc has cleared up any "conflict".

Bottom line; work together towards a common goal; improving the availability of development tools on AOS4?

Last edited by Overflow on 10-Jan-2015 at 11:19 PM.
Last edited by Overflow on 10-Jan-2015 at 11:19 PM.

@Daytona675x

I think that ASiegel meant "code review" to be "check that the code matches the bounty description" rather than code review in the more traditional quality assurance sense. Basically, it is the AmiDARK source code? And does it have all of the mentioned features?

Please try to be a bit more diplomatic in your critique of other people's work. Talking about what could be improved rather than "what you're doing wrong" does wonders in helping people to accept critique instead of getting offended and defensive.


@AmiDARK

Try not to take Daytona675x's comments personally, even though he comes across rather blunt (and a bit condescending). He has found a range of things that need to be fixed and/or could be improved. Take it as a chance to improve the quality of the code, and to learn from a very talented software developer. When peer code review is done properly (i.e., constructively), we can all learn from each other and collectively become better developers.


@all

Probably the best way to confirm that the bounty's requirments have been filled, would be for a few people to have a go at compiling the released code, and testing the examples. We'll need at least one person to build and test it on each supported platform: AmigaOS, MorphOS, and AROS. It it compiles on all platforms, the examples run, and the list of supported features work, then the bounty's requirements have been met.

Right now, I don't have time to check it out, but I'm sure that we have enough developers to get this done.

Actually, it would be interesting to get a complete rookie to download the code and try to build it. After all, this game engine is geared toward less experienced programmers. This would help identify key items that need to be improved to help beginners get started.

Hans

Last edited by Hans on 11-Jan-2015 at 12:15 AM.

_________________
http://hdrlab.org.nz/ - Amiga OS 4 projects, programming articles and more. Home of the RadeonHD driver for Amiga OS 4.x project.
https://keasigmadelta.com/ - More of my work.

@AmiDark
Exactly why I don't want to share my code (except with the AmiOuest team). To be reviewed by really professional developpers is a suicide...

@Hans
I tried to compile it under OS4, but no luck, CodeBench project failed (AmiDark notified)
And like you, I have others things to do (ztools and FE compatibility)

_________________
http://apps.amistore.net/zTools

@Hans
"a complete rookie to download the code"

Sorry, Hans, but did you downloaded the previous beta versions ? If yes, did you still tried to compile a sample ?

Me, yes... Sure, I'm a rookie, but it was not easier than that...

_________________
http://apps.amistore.net/zTools

@zzd10h

Quote:

zzd10h wrote: @Hans "a complete rookie to download the code" Sorry, Hans, but did you downloaded the previous beta versions ? If yes, did you still tried to compile a sample ? Me, yes... Sure, I'm a rookie, but it was not easier than that...

I haven't tried the latest versions. However, it doesn't matter how easy I find it as I'm an experienced developer and could resolve any problems fairly easily. What looks easy for me may be difficult for a beginner.

What really matters is how easily a complete beginner can go from download through to compiling and running the examples (and beyond). That process needs to be as easy as possible, and it can be difficult for experienced people to guess what beginners will have trouble with.

Hans

_________________
http://hdrlab.org.nz/ - Amiga OS 4 projects, programming articles and more. Home of the RadeonHD driver for Amiga OS 4.x project.
https://keasigmadelta.com/ - More of my work.