Jump to content

  • Log In with Google      Sign In   
  • Create Account

Request for Peer Reviews

  • You cannot reply to this topic
12 replies to this topic

#1 Buckeye   Crossbones+   -  Reputation: 6298

Like
0Likes
Like

Posted 17 March 2014 - 08:03 AM

My article A Blending Animation Controller for a Skinned Mesh is in need of some peer reviews. I fully understand it's only been published a week and reviews can take a while. It does have some "up" votes and unbird has marked it peer reviewed, but, as yet, only one comment. The process of animating a skinned mesh has been asked about a bit in the forums lately and the article may provide some insight.

 

Just asking. I would appreciate any support.


Edited by jbadams, 19 March 2014 - 03:42 AM.
Added link to article.

Please don't PM me with questions. Post them in the forums for everyone's benefit, and I can embarrass myself publicly.


Sponsor:

#2 jbadams   Senior Staff   -  Reputation: 19366

Like
3Likes
Like

Posted 19 March 2014 - 03:48 AM

I don't feel sufficiently qualified in the field to judge your article, but I've shared your request on Facebook for you.

 

Thanks for contributing, and hopefully someone can help you out with some peer review!



#3 frob   Moderators   -  Reputation: 22731

Like
3Likes
Like

Posted 19 March 2014 - 03:27 PM

Interesting reading, and consistent with everything I've used.  Marked as reviewed.

 

One thing that stood out as a problem since it comes from a veteran programmer: you have a public nonvirtual destructor in the c++ code.  dry.png   Public and virtual, or protected and non-virtual, those are the options for c++ destructors. The rest of the code looks like you stripped it down as well. The article passes but just don't check in the code. There are probably going to be beginners copying the code and they won't know why it matters, so it is something of a convenience I would recommend fixing to make the world a better place.


Check out my book, Game Development with Unity, aimed at beginners who want to build fun games fast.

Also check out my personal website at bryanwagstaff.com, where I write about assorted stuff.


#4 Servant of the Lord   Crossbones+   -  Reputation: 21008

Like
0Likes
Like

Posted 19 March 2014 - 08:14 PM

One thing that stood out as a problem since it comes from a veteran programmer: you have a public nonvirtual destructor in the c++ code.  dry.png   Public and virtual, or protected and non-virtual, those are the options for c++ destructors.

Even on non-abstract classes that aren't intended to be inherited? huh.png 

If the destructor is protected, doesn't that mean you can't use the class directly (since it can't be destroyed)? And even if protected, it could still be inherited and do damage anyway (protected doesn't stop it from being inherited, which is the danger of non-virtual destructors).

And if the class doesn't have any virtual functions, and is intended to be used directly, then adding a virtual destructor unnecessarily adds a vtable, doesn't it?

It seems like a better habit, if you must enforce one, would be: Declare your destructor virtual (if you intend it or expect it to be inherited) or declare your class final (to prevent inheritance).
 
"Public and virtual" -> Unnecessary if not inherited, necessary if inherited.
"Protected and non-virtual" -> What protection does that give?
 
Something seems to be going over my head here. wacko.png Would you mind elaborating?

(Perhaps my question + your explanation should be split into its own thread in the General Programming subforum if the answer is technical)


Edited by Servant of the Lord, 19 March 2014 - 08:21 PM.

It's perfectly fine to abbreviate my username to 'Servant' rather than copy+pasting it all the time.
All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.
Of Stranger Flames - [indie turn-based rpg set in a para-historical French colony] | Indie RPG development journal

[Fly with me on Twitter] [Google+] [My broken website]

[Need web hosting? I personally like A Small Orange]


#5 frob   Moderators   -  Reputation: 22731

Like
0Likes
Like

Posted 20 March 2014 - 12:13 AM

Agreed with you. Split the topic here.


Check out my book, Game Development with Unity, aimed at beginners who want to build fun games fast.

Also check out my personal website at bryanwagstaff.com, where I write about assorted stuff.


#6 Hodgman   Moderators   -  Reputation: 31822

Like
0Likes
Like

Posted 20 March 2014 - 12:39 AM

you have a public nonvirtual destructor in the c++ code

That's fine as long as AnimController isn't meant to ever be used as a base class. A bigger issue is that AnimController has a Rule of Three violation.

[edit]Actually, the fact that AnimController's members are protected says that it is meant to be used a base class... Was this the intention?


Edited by Hodgman, 20 March 2014 - 01:56 AM.


#7 Buckeye   Crossbones+   -  Reputation: 6298

Like
1Likes
Like

Posted 21 March 2014 - 02:20 PM

Yoiks. The code in the article is certainly not a good example of proper C++ class implementation. Mea culpa.

 

The destructor is, in fact, empty. The class makes no memory allocations, and no explicit deletions or releases need to be made. The only pointer used is a pointer to the frame hierarchy which is owned by someone else and "given" to the animation controller to check consistency between animation frame-names and hierarchy frame-names, and which provides access to store the results in that hierarchy.

 

The pointers used in some of the function calls are used to return values and don't represent allocated objects.

 

 

 


AnimController's members are protected says that it is meant to be used a base class... Was this the intention?

Nope. Just bad style, I'm afraid.

 

I'll get rid of the destructor and the protected status.

 

EDIT: Revised as indicated, and notes regarding the Rule of Three added as a caveat should an implementation of the class need to add a destructor.


Edited by Buckeye, 21 March 2014 - 02:41 PM.

Please don't PM me with questions. Post them in the forums for everyone's benefit, and I can embarrass myself publicly.


#8 Rld_   Members   -  Reputation: 1520

Like
0Likes
Like

Posted 08 May 2014 - 02:33 AM

Instead of making a new thread, I hope it's ok to post in here as it is regarding the same thing, but for this article: http://www.gamedev.net/page/resources/_/technical/graphics-programming-and-theory/let-there-be-shadow-r3642

 

It's been out there for not that long, but considering it's a Unity related topic my guess is that not a lot of people have that much experience with it concerning the shader specifics. It has one passed review from Yourself, no comments and a couple of upvotes. I would also like to know if there is something I can do myself to give it some more attention for peer reviews.

 

Let me know if I need to make this in its own topic. :)



#9 jbadams   Senior Staff   -  Reputation: 19366

Like
0Likes
Like

Posted 08 May 2014 - 03:58 AM

Sent out a quick shout-out for you! :)

 

https://www.facebook.com/GameDev.net/posts/10152020827152443



#10 Rld_   Members   -  Reputation: 1520

Like
0Likes
Like

Posted 08 May 2014 - 04:02 AM

Sent out a quick shout-out for you! smile.png

 

https://www.facebook.com/GameDev.net/posts/10152020827152443

Thanks a lot! :)



#11 Rld_   Members   -  Reputation: 1520

Like
0Likes
Like

Posted 15 May 2014 - 01:35 AM

I was wondering, in my case it seems people do not know enough about unity (and shaders related to unity) to mark my article as peer reviewed (except for 1 by Yourself) and I am not sure if it will get more.

 

What will happen in that case? 



#12 Gaiiden   Senior Staff   -  Reputation: 5274

Like
0Likes
Like

Posted 15 May 2014 - 07:45 PM

An article that isn't peer reviewed currently isn't really handled any differently than an article that is. We did have plans to make the peer reviewed articles float higher in search results and stuff, but I don't know if that was implemented. Really it's the perception readers get - an article that hasn't been peer reviewed should be approached with a certain amount of caution - even the best of us can't always get it all right all the time. It doesn't mean the article isn't of some good level of quality, it simply hasn't been properly checked over for any errors that may exist within (mainly technical - I usually get all the major grammatical/spelling stuff).

 

So don't worry about it being taken down after x number of weeks without peer review or anything like that.


Drew Sikora
Executive Producer
GameDev.net


#13 analivia   Members   -  Reputation: 105

Like
0Likes
Like

Posted 07 August 2014 - 03:32 AM

i am an article writer but cant write on games

http://jogarjogosdemoto.com/


Edited by analivia, 10 August 2014 - 03:09 AM.






PARTNERS