Poster | Thread |
klx300r
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 6:53:56
| | [ #121 ] |
|
|
|
Elite Member |
Joined: 4-Mar-2008 Posts: 3843
From: Toronto, Canada | | |
|
| Hans wrote:
"P.S. Why is whether he deserves to receive the bounty money still even being discussed? He's delivered what he promised. Just release the funds to him already."
-----------------------------------------------------------------------------------
+1, I donated to the bounty as well and I fully agree! Open Sourcing it was a great idea so that more talented developers can co-operate and bring it to the next level
_________________ ____________________________ c64-2sids, A1000, A1200T-060@50(finally working!),A4000-CSMKIII ! My Master Miggies- Amiga 1000 & AmigaOne X1000 ! mancave-ramblings X1000 I BELIEVE |
|
Status: Offline |
|
|
zzd10h
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 7:11:48
| | [ #122 ] |
|
|
|
Amiga Developer Team |
Joined: 21-May-2012 Posts: 1077
From: France | | |
|
| |
Status: Offline |
|
|
Bugala
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 7:17:17
| | [ #123 ] |
|
|
|
Cult Member |
Joined: 21-Aug-2007 Posts: 654
From: Finland | | |
|
| @Bug pointers
Since it seems quite many of this topics comments are still about bugs, I am just wanting to remind you that this is the correct place to put them: http://amigaworld.net/modules/newbb/viewtopic.php?topic_id=39807&forum=15&0
If someone wants to help AmiDARK fixing the bugs, it will be very helpful if all the bug reports can be found from one same thread, instead from a thread that has also commetns regarding something else in it, making interested one need to skim through all the unrelated comments.
@Asiegel
I think it would be good if you could post this link to the original message by which you started this topic to make it easy for people to find it, since right now the bug thread (receiving no messages) is out of the main page, while this, still full of lively discussion, gets to front page all the time making it tempting and natural to want to post bug reports to this thread instead, since the bug report thread is bit hard to find. |
|
Status: Offline |
|
|
ASiegel
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 9:47:43
| | [ #124 ] |
|
|
|
Regular Member |
Joined: 22-Oct-2013 Posts: 212
From: Unknown | | |
|
| @Bugala
Quote:
I think it would be good if you could post this link to the original message by which you started this topic to make it easy for people to find it |
The post has been updated accordingly. |
|
Status: Offline |
|
|
ASiegel
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 10:04:24
| | [ #125 ] |
|
|
|
Regular Member |
Joined: 22-Oct-2013 Posts: 212
From: Unknown | | |
|
| @Hans
Quote:
P.S. Why is whether he deserves to receive the bounty money still even being discussed? He's delivered what he promised. Just release the funds to him already. |
We appreciate your feedback.
However, considering that we are asking volunteers, who usually have to work for a living and other important commitments, to review a project in their free time, I think it is appropriate to wait a minimum of two weekends for people to provide feedback, which is what happened with most Power2People projects and has proven to be a useful approach.
|
|
Status: Offline |
|
|
ASiegel
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 10:10:22
| | [ #126 ] |
|
|
|
Regular Member |
Joined: 22-Oct-2013 Posts: 212
From: Unknown | | |
|
| @zzd10h
Quote:
What will happen if the bounty is not accepted ?
To be more precise, where will be refunded the money of the backers if they don't want to fund others power2people bounties ?
Thank you for your reply. |
In case a project is cancelled, donors can simply ask for a refund.
(Depending on when the initial payment was made, Paypal may refuse to reimburse the paid fees so a donor would receive less than the originally paid amount. Paypal only allows for full refunds for up to 60 days after a payment is made.) |
|
Status: Offline |
|
|
AmiDARK
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 10:12:41
| | [ #127 ] |
|
|
|
Regular Member |
Joined: 28-Mar-2007 Posts: 469
From: South France | | |
|
| @ASiegel Thank you for your answer André.
During this remaining "delay", I will continue to show my "will" by continuying to fix the bugs founded on the Free Time I can allocate to the project regarding to the global situation. As many bugs should be fast to fix, on next tuesday, I can allocate few hours for these issues/improvements/bugs if I didn't find enough time to fix them before.
Regards, AmiDARK |
|
Status: Offline |
|
|
zzd10h
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 11:26:58
| | [ #128 ] |
|
|
|
Amiga Developer Team |
Joined: 21-May-2012 Posts: 1077
From: France | | |
|
| |
Status: Offline |
|
|
Yssing
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 12:18:14
| | [ #129 ] |
|
|
|
Super Member |
Joined: 24-Apr-2003 Posts: 1101
From: Unknown | | |
|
| Since we are all adults, it should be possible, no matter the intentions, to choose the words more carefully. _________________
|
|
Status: Offline |
|
|
Robert
| |
Re: Request for review: AmiDark Source Code Posted on 16-Jan-2015 12:48:33
| | [ #130 ] |
|
|
|
Cult Member |
Joined: 10-Mar-2003 Posts: 879
From: Glasgow | | |
|
| @Daytona675x
Quote:
Here's the next chapter of my egoistic but fair code-review |
FWIW and as a mere observer, I found that very informative, thank you._________________ Robert -- A1XE G4, OS4.1. Peg1 G3, MOS 1.4. Abel Soul - Check out our tunes on Spotify |
|
Status: Offline |
|
|
zzd10h
| |
Re: Request for review: AmiDark Source Code Posted on 17-Jan-2015 9:27:03
| | [ #131 ] |
|
|
|
Amiga Developer Team |
Joined: 21-May-2012 Posts: 1077
From: France | | |
|
| @all
A lot of speak but very few real testers (except LionStorm)
Therefore, I replugged my 9250 to test myself.
AmiDark sent me a new library a new BoingBall sample
-a 640x480 screenmode is REALLY required -my latest BoingBall binary freezes my system -my latest BoingBall binary compiled with your latest libAmiDark.a freezes my system
The one from AmiDark works :
Last edited by zzd10h on 17-Jan-2015 at 01:28 PM. Last edited by zzd10h on 17-Jan-2015 at 09:27 AM.
_________________ http://apps.amistore.net/zTools |
|
Status: Offline |
|
|
Daytona675x
| |
Re: Request for review: AmiDark Source Code Posted on 17-Jan-2015 13:10:55
| | [ #132 ] |
|
|
|
Regular Member |
Joined: 5-Jan-2011 Posts: 491
From: Germany | | |
|
| System
Not much in here since all functions in System_CheckLists.c are not available yet.
all functions in System_BitWise.c (deg. 2) Such functions should be inline.
all functions in System_BitWise.c (deg. 1) BitID should be unsigned. And instead of adding redundant functions like SetBit vs. BitSet, which do exactly the same, it would certainly be better to add functions that support different types of "var" instead: SetBit(int*, unsigned int); SetBit(unsigned int*,unsigned int); And if those different names were added to provide compatibility to DarkBasic: consider trashing them since it simply is a bad idea (and it is not as if you could just copy'n'paste DarkBasic code, you have to make some adjustments anyway, so you can as well adjust things like that). But if you insist on keeping it then make'em inline and don't reimplement them, instead implement it once (for example in SetBit) and then let the variants with the other names like BitSet call SetBit.
BitTest, TestBit (deg. 1, 2) System_BitWise.c Both functions should be reduced to one line (41 respective 51). And BitID should be made unsigned, as already been said.
all functions in System_Keyboard.c (deg. 1, 2) Such functions should be inline. And those different naming variants don't make sense, for example DEDisableEscapeKey vs. DEDisableEscapekey. And if you add such useless redundant functions then you should at least be consistent: I'd expect to see DEDisableSystemKeys and DEDisableSystemkeys then, but the latter isn't there. Really: remove those redundant functions, it doesn't make sense. And it doesn't really help anybody. C simple isn't case-insensitive and you can't trick it into being it that way.
DESystemDMEMAvailable, DESystemDmemAvailable (deg. 3, critical, unexpected behaviour) System_Memory.c, lines 52-55, 73-76 This function always returns 0. But, according to the docs, it should work and return the total video memory available on the system. Since a program might want to first check whether there is enough video memory available for its task by using this function it may then decide to exit immediately because of this function not working as it should. Therefore this can be considered a "degree 3"-bug.
System_Variables.c in general (deg. 3 and 1) Some of the global variables are initialized here, some are somewhere else, some are not at all. Hint: A global variable with a name like ci can be pretty dangerous, especially if it is extern like this one. Note: since those variables in question are used by the unfinished System_CheckLists-functions it is uncritical at the moment. The severity-degree above is the one this issue would have if this wasn't fixed when those functions go "online". It is strongly suggested to fix those asap.
DEExitPrompt (deg. 3, critical, root for undefined behaviour) System_Functions.c, lines 38-40, 45-47) Depending on the length of pString / pString2 the CopyMemory commands will not copy the 0-terminator-characters, thus leaving the global strings ExitPrompt1 and ExitPrompt2 in an unterminated state. That again is the root for undefined behaviour as soon as any of those gets used.
SYSTEM_Constructor (deg. 3) System_Constructors.c, lines 32-37 checklist and MyCheckList are not initialized. Actually ci, checklistexists, checklisthasvalues, checklisthasstrings, checklistqty are neither, but those are at least (inconsitently) initialialized in System_Variables.c at their definition. That MyCheckList is not initialized is not critical at the moment, since currently it's being adjusted before each use by a call to INTERNAL_CKLUpdatePTR. It is nevertheless not good and should be corrected. For checklist however I cannot really tell you yet whether this is / will be critical or not. But in general it is always bad to leave things in an unitialized state (after all things like that are one of the main reasons why the engine and all demos are unstable). Note: As being said initially all those System_CheckLists.c functions are not available yet. Therefore this is not critical yet. But since things tend to be forgotten I guess it's best to fix that now. The severity-degree above is the one this issue would have if this wasn't fixed when those functions go "online". It is strongly suggested to fix those asap.
_________________ 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 |
|
Status: Offline |
|
|
Tomppeli
| |
Re: Request for review: AmiDark Source Code Posted on 17-Jan-2015 13:59:41
| | [ #133 ] |
|
|
|
Super Member |
Joined: 18-Jun-2004 Posts: 1652
From: Home land of Santa, sauna, sisu and salmiakki | | |
|
| @Daytona675x
Have you fixed already all those issues you have listed here ? Isn't it open source now.
@ASiegel Thanks, for clarification.
Last edited by Tomppeli on 17-Jan-2015 at 02:00 PM.
_________________ Rock lobster bit me. My Workbench has always preferences. X1000 + AmigaOS4.1 FE "Anyone can build a fast CPU. The trick is to build a fast system." -Seymour Cray |
|
Status: Offline |
|
|
Overflow
| |
Re: Request for review: AmiDark Source Code Posted on 17-Jan-2015 14:01:53
| | [ #134 ] |
|
|
|
Super Member |
Joined: 12-Jun-2012 Posts: 1628
From: Norway | | |
|
| Can people please keep the thread AmiDark created for Engine bug reports and feedback clean of discussions regarding the value of the engine, state and hairsplitting of sentances?
It would be brilliant that one thread is reserved for objective engine focused discussion, and let this thread serve as a thread for the more ...subjective tangents/offshots.
I had hopes that the new thread would be spared of going off the rails, but its heading into trainwreck territory already.
|
|
Status: Offline |
|
|
Daytona675x
| |
Re: Request for review: AmiDark Source Code Posted on 17-Jan-2015 14:04:07
| | [ #135 ] |
|
|
|
Regular Member |
Joined: 5-Jan-2011 Posts: 491
From: Germany | | |
|
| @Tomppeli Quote:
Have you fixed already all those issues you have listed here ? |
No, I'm just reviewing the source.
@Overflow Quote:
Can people please keep the thread AmiDark created for Engine bug reports and feedback clean of discussions regarding the value of the engine, state and hairsplitting of sentances |
Please let's not forget that it was AmiDark himself who put the train off track on his debug-topic when he suddenly edited his initial post and came up with his weird leadership etc. stuff... Something that deserves an explanation.Last edited by Daytona675x on 17-Jan-2015 at 02:09 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 |
|
Status: Offline |
|
|
Overflow
| |
Re: Request for review: AmiDark Source Code Posted on 17-Jan-2015 14:33:49
| | [ #136 ] |
|
|
|
Super Member |
Joined: 12-Jun-2012 Posts: 1628
From: Norway | | |
|
| @Daytona675x
Well, since it was opensourced thru the bounty, the wording didnt mean a whole lot per see.
The review is asking him to fix the weaknesses or add missing features, so in that regard hes the "leader" since hes the only one actually doing any developing of the engine (ofcourse thanks to the reviewers).
Additionally, its possible to take a step back and think "do we need 2 threads with the same content, or should we keep that seperated?" You already know the reaction pattern by now.
It would be great if his original post contained a "ON topic posts Only", just to make it easier for Moderators to keep the thread clean.
Other than that; I obviously appriciate your review efforts |
|
Status: Offline |
|
|
AmiDARK
| |
Re: Request for review: AmiDark Source Code Posted on 17-Jan-2015 20:35:20
| | [ #137 ] |
|
|
|
Regular Member |
Joined: 28-Mar-2007 Posts: 469
From: South France | | |
|
| @Overflow & Daytona675x : You should read carefully that it was stated "until Power2People pay the bounty". Even if I uploaded the engine on an open source website to allow the review. The contract between me and power2people is NOT YET completed and I'M NOT YET PAID for the release. That mean in term of contract, that the engine CANNOT be considered as MPL Open Source, even if it's available for "reviewing". In that case, I am legally the owner of the source code and I can decide everything I want ... If people read carefully my message and follow the changes, the statement "until Power2 people pay the bounty" was available from the 1st version of the message ... So far before Daytona put it's 1st answer to the thread. No manipulation .. Only the 3rd rule was added after, just to show Daytona he didn't read carefully my messages ...
@Daytona675x : Thank you for taking in consideration the 3 degree I have explained for debug. I think it's important that bug that causes crash are the most important to detect and fix :) But, to be honest, your review is more a "debug" than a review as it was stated in the bounty that AmiDARK Engine is not finished. Even A.Siegel states that bug report should be reported on the other thread and by continuing do bug report here, you don't respect Power2People request and are, in this thread "out of subject". But I understand that apparently, you don't know what it is to "review an unfinished product", as you clearly stated that "an incomplete product may never be sell"... That mean you never reviewed an unfinished product before ...
However, Thank you to you for your bug report, special thank you to BSZili that started make changes and fixs to the engine, and to everyone that help in any way.
Regards, AmiDARK Last edited by AmiDARK on 17-Jan-2015 at 08:45 PM.
|
|
Status: Offline |
|
|