Java-Gaming.org    
Featured games (79)
games approved by the League of Dukes
Games in Showcase (477)
Games in Android Showcase (108)
games submitted by our members
Games in WIP (536)
games currently in development
News: Read the Java Gaming Resources, or peek at the official Java tutorials
 
    Home     Help   Search   Login   Register   
Pages: [1] 2
  ignore  |  Print  
  Node.sharedCopy() buggy?  (Read 3785 times)
0 Members and 1 Guest are viewing this topic.
Offline croft

Junior Member




Java, Java, Java


« Posted 2005-10-11 00:37:16 »

I've spent a couple of days debugging and now I suspect that Node.sharedCopy() is buggy.

Here is the suspect code in Node.sharedCopy():
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
14  
        } else if (node instanceof Shape3D) {

            Shape3D shape = (Shape3D)node;
            Shape3D newShape = new Shape3D();
            newShape.setAppearance(shape.getAppearance());
            newShape.setGeometry(shape.getGeometry());

            newShape.setBoundsAutoCompute(false);
            newShape.setBounds(shape.getBounds());
            newShape.updateBounds(false);
            newShape.setPickable(node.getPickable());
            newShape.setPickable(node.getRenderable());

            return newShape;


My problems go away when I use something much simpler like this instead:
1  
2  
3  
       Shape3D  clone = new Shape3D (
         shape3D.getGeometry   ( ),
         shape3D.getAppearance ( ) );


1.  Is "newShape.setPickable(node.getRenderable());" supposed to be "newShape.setRenderable(node.getRenderable());"?

2.  Why does Node.sharedCopy() turn off bounds updating?

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #1 - Posted 2005-10-11 13:10:13 »

1) looks like a bug

2) I'm not sure

Does it make a difference which way it is initialised?  I havn't had problems with it in the past.

Will.

Offline croft

Junior Member




Java, Java, Java


« Reply #2 - Posted 2005-10-11 20:10:00 »

Does it make a difference which way it is initialised?  I havn't had problems with it in the past.

Let's agree to fix the first bug just because it is obviously an error.  Is the original author available to confirm?

Let's discuss fixing the second bug.  The code duplicates the node bounds in the shared copy.  This means that the shared copy might not be rendered unless it is in the same position as the original.  When would you ever create one or more shared copies with the same position as the original?

This bug is truly mystifying as it makes the shared copy invisible since the geometry and the bounds do not overlap.  This bug cost me two days of debugging because it did not work as I expected it to.  Most of that time I thought the problem was in my code.  Let's fix this bug by deleting or remarking out these lines:
1  
2  
3  
            newShape.setBoundsAutoCompute(false);
            newShape.setBounds(shape.getBounds());
            newShape.updateBounds(false);


David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline croft

Junior Member




Java, Java, Java


« Reply #3 - Posted 2005-10-11 21:49:08 »

Does it make a difference which way it is initialised?  I havn't had problems with it in the past.

One way we could bypass the issue as to whether to fix sharedCopy() or not is to implement cloneNode() which is currently missing from the Xith implementation.  We could deprecate sharedCopy() but keep it there for those currently using it.

Here is the javadoc for cloneNode():
http://java.sun.com/products/java-media/3D/forDevelopers/J3D_1_3_API/j3dapi/javax/media/j3d/Shape3D.html#cloneNode(boolean)

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline hawkwind

Junior Member




Java games rock!


« Reply #4 - Posted 2005-10-12 01:08:25 »

I hate the idea of leaving bad code around for someone else to step in.  I say fix it.  I would not be to concerned with ownership of the original developer.  You have made a could case for fixing this code.
Offline arne

Senior Member




money is the worst drug- we should not let it rule


« Reply #5 - Posted 2005-10-12 01:19:36 »

Yeah fix it - and maybe add a note in the javadoc. That they should, if the old code should keep working, do that and that.
OR write another function (e.g. cloneNode) and mention it in the javadoc, then nobody can step in that easily anymore and others can still use the old code.

:: JOODE :: Xith3d :: OdeJava ::
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #6 - Posted 2005-10-12 02:47:11 »

But:

Does it make a difference which way it is initialised anyway?  I havn't had problems with it in the past.

I would like to confirm the existance of the problem before worrying about the work around.  As far as I can see there is one obvious bug, a second possible bug - the order of initialisation *shouldn't* matter (though it might), and I'm not quite sure why sharedCopy is disabling boundsUpdating and what the proposal is to do instead?

Will.

Offline croft

Junior Member




Java, Java, Java


« Reply #7 - Posted 2005-10-12 20:18:38 »

Does it make a difference which way it is initialised anyway?  I havn't had problems with it in the past.
Based on my experience of two days debugging, it does.  The initialization does not act as one would expect it to.  Here is the javadoc for the code in question:
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
    /**
     * Creates a shared copy of the given Node.
     * A shared copy is one where the geometry and appearance is shared, but all other
     * nodes are copied.  This is a replacement for shared groups because of
     * performance considerations.  If you are loading the same model many times
     * then this can save on memory and load times.  The only allowable within the subtree
     * are groups and shapes. This also copies a shapes bounds and turns autocomute off
     * so that it is fast to insert the model into the scene.
     *
     * @param node The Node to be copied
     * @return a shared copy of the given Node
     */

 


Based on the above, it looks like bounds are copied and fixed for speed of insertion.  I'm not sure this is justified given the confusion it causes.  Is the user of this method supposed to know that they must then reenable bounds if they want their shared copies to have non-identical locations?

The two authors of Node.java are Scott Shaver and David Yazel.  I understand that David Yazel is no longer around but how about Scott Shaver?

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #8 - Posted 2005-10-18 01:11:54 »

Thanks for the info.  David was definitally the author of that code, I don't think Scott is the best to ask, so it's just us Smiley

I think I agree that the confusion over bounds isn't worth the speed improvement, at the very least we can override the method and give the user the option to turn it off (if by defualt we turned it on).

Do you think many people would be affected should we change the initialisation, fix the other bug and turn on bounds calculation?  It seems to me this is a viable option.

Will.

Offline croft

Junior Member




Java, Java, Java


« Reply #9 - Posted 2005-10-18 20:04:00 »

Do you think many people would be affected should we change the initialisation, fix the other bug and turn on bounds calculation?  It seems to me this is a viable option.

How many do you suppose are currently using sharedCopy()?  I suspect it cannot be many or these bugs would have been flushed out earlier.  Of those that are using it, I would guess that they must be re-enabling bounds manually after initialization.

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline croft

Junior Member




Java, Java, Java


« Reply #10 - Posted 2005-10-20 20:16:35 »

How many do you suppose are currently using sharedCopy()?  I suspect it cannot be many or these bugs would have been flushed out earlier.  Of those that are using it, I would guess that they must be re-enabling bounds manually after initialization.

I thought about it overnight.  Let's leave sharedCopy() unchanged but deprecate it and implement the missing cloneNode() method.

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline croft

Junior Member




Java, Java, Java


« Reply #11 - Posted 2005-10-24 21:21:17 »

I've spent a couple of days debugging and now I suspect that Node.sharedCopy() is buggy.

Where are we on this?  Are we ready to proceed with the bugfixes?

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline arne

Senior Member




money is the worst drug- we should not let it rule


« Reply #12 - Posted 2005-10-24 21:31:51 »

+1 for cloneNode

:: JOODE :: Xith3d :: OdeJava ::
Offline hawkwind

Junior Member




Java games rock!


« Reply #13 - Posted 2005-10-25 23:58:52 »

+1 for cloneNode
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #14 - Posted 2005-10-27 08:12:10 »

Yes, lets do it.  I am insanely busy this week, but have some time on the weekend to make/merge any changes.

Will.

Offline croft

Junior Member




Java, Java, Java


« Reply #15 - Posted 2005-10-29 00:27:25 »

Yes, lets do it.  I am insanely busy this week, but have some time on the weekend to make/merge any changes.

OK, what do you think of this as a first draft in an incremental approach?

1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
14  
15  
16  
17  
18  
19  
    public Node cloneNode(boolean forceDuplicate)
    {
      try
      {
        Node  node = ( Node ) getClass ( ).newInstance ( );
     
        node.duplicateNode(this,forceDuplicate);
     
        return node;
      }
      catch ( Exception  ex )
      {
        RuntimeException  runtimeException = new RuntimeException ( );
       
        runtimeException.initCause ( ex );
       
        throw runtimeException;
      }
    }


This is based on the Sun javadoc for cloneNode().

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline arne

Senior Member




money is the worst drug- we should not let it rule


« Reply #16 - Posted 2005-10-29 19:14:17 »

What does
1  
node.duplicateNode(this,forceDuplicate);

do?

Do we have to implement it, or does it work correctly by default? Have you tested it?

Arne

:: JOODE :: Xith3d :: OdeJava ::
Offline croft

Junior Member




Java, Java, Java


« Reply #17 - Posted 2005-10-31 22:15:02 »

What does
1  
node.duplicateNode(this,forceDuplicate);

do?

Do we have to implement it, or does it work correctly by default? Have you tested it?


We would have to implement it.  It would be a next step in this incremental approach.

The Xith implementation of class Node currently has a duplicateNode() method as an empty placeholder.  After we write cloneNode(), we would need to tackle duplicateNode() and cloneTree() as well.

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #18 - Posted 2005-11-01 00:44:08 »

Fair enough, looks good.

Will.

Offline croft

Junior Member




Java, Java, Java


« Reply #19 - Posted 2005-11-01 20:00:44 »

Fair enough, looks good.

OK, what is the next step?  Do we update the code now?

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #20 - Posted 2005-11-13 05:28:13 »

Sure, code it up and I will commit it in.  Submit changes as a either the changed files or a patch to IssueZilla.  Prefrably, keep to the Xith3D coding standards.

Cheers,

Will.

Offline croft

Junior Member




Java, Java, Java


« Reply #21 - Posted 2005-11-21 21:49:09 »

Sure, code it up and I will commit it in.  Submit changes as a either the changed files or a patch to IssueZilla.  Prefrably, keep to the Xith3D coding standards.

OK, this was the first time I have used IssueZilla.  I think I botched the issue description but I did figure out how to submit the updated source code file as an attachment:
https://xith3d.dev.java.net/issues/show_bug.cgi?id=102

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #22 - Posted 2005-11-30 13:51:20 »

OK, I added it in.  I ended up just copy+pasting the code from the description since that was the only change.  It was good to have the .java file though to see it in context, so thanks for doing that.

Please submit the other changes when ready, I will try to commit them within a week.  If there's some code people can experiment with, I generally make an "experimental" build when I am making changes so people can play with it before it goes into the core.  You're welcome to do the same if you think its worth it.

Cheers,

Will.

Offline croft

Junior Member




Java, Java, Java


« Reply #23 - Posted 2005-12-03 20:18:57 »

OK, I added it in.  I ended up just copy+pasting the code from the description since that was the only change.  It was good to have the .java file though to see it in context, so thanks for doing that.


I made a couple of other changes to the file which did not get committed.  Please add my @author tag so I can get credit for my contribution.  Also, I updated the copyright statement from "Copyright (c) 2003, Xith3D Project Group" to "Copyright (c) 2003-2005, Xith3D Project Group".  Is everyone who is a contributor a member of the Xith3D Project Group?


Please submit the other changes when ready, I will try to commit them within a week.  If there's some code people can experiment with, I generally make an "experimental" build when I am making changes so people can play with it before it goes into the core.  You're welcome to do the same if you think its worth it.

OK, I will probably work on Node.duplicateNode() next.  I will probably not make an experimental build because no one is using the methods I am working on since they were not previously implemented.

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #24 - Posted 2005-12-04 14:21:12 »

OK, I added it in.  I ended up just copy+pasting the code from the description since that was the only change.  It was good to have the .java file though to see it in context, so thanks for doing that.


I made a couple of other changes to the file which did not get committed.  Please add my @author tag so I can get credit for my contribution.  Also, I updated the copyright statement from "Copyright (c) 2003, Xith3D Project Group" to "Copyright (c) 2003-2005, Xith3D Project Group".  Is everyone who is a contributor a member of the Xith3D Project Group?

Ok, I didn't pick that up.  Will there be a lot of changes to that class?  I ask because generally file level author attributions are added for major additions/changes to the class in question so that the list is one of the major authors of the class to keep the list size down (you will notice my name appears only in files which I have majorly modified and not all I have modified).  All contributors are listed on the Xith.org page and this is the official credits (and you will be recognised there).  The dates should be updated, indeed.  As for your last question, I'm not really sure as I didn't write that myself.  Essentially it's a BSD licensed community project, so I'm not sure what difference it makes (legally there is no "Xith3D Project Group" entity that I know of).  Do you have a specific concern?


Quote


Please submit the other changes when ready, I will try to commit them within a week.  If there's some code people can experiment with, I generally make an "experimental" build when I am making changes so people can play with it before it goes into the core.  You're welcome to do the same if you think its worth it.

OK, I will probably work on Node.duplicateNode() next.  I will probably not make an experimental build because no one is using the methods I am working on since they were not previously implemented.


OK.  After duplicateNode what else is there to do for this feature?

Thanks for your contribution!

Cheers,

Will.

Offline croft

Junior Member




Java, Java, Java


« Reply #25 - Posted 2005-12-05 02:38:36 »

Ok, I didn't pick that up.  Will there be a lot of changes to that class?  I ask because generally file level author attributions are added for major additions/changes to the class in question so that the list is one of the major authors of the class to keep the list size down (you will notice my name appears only in files which I have majorly modified and not all I have modified).  All contributors are listed on the Xith.org page and this is the official credits (and you will be recognised there).  The dates should be updated, indeed. 

I go by the policy that anyone who contributes any code whatsoever to a class gets an @author tag attribution.  It cuts both ways in that it gives credit where credit is due and, in the case of less stellar work, does not assign blame to the original authors when the actual change might have been made by an unattributed individual.  I will even go so far as to put an @author tag for "Unattributed" when an author is unknown or anonymous.  Here is what I wrote about it a few years ago:

"All developers who modify source code, for any reason whatsoever, should place their javadoc "@author" tag in the file and update the version date. This both alerts the other developers that changes in behavior may be due to modifications by other developers, provides credit where credit is due, and allows the code owner to track down those with experience and knowledge with certain code sections that may need debugging or enhancement. The latter is especially important with legacy source code and projects with high developer turnover."
http://croftsoft.com/library/tutorials/version/

As for your last question, I'm not really sure as I didn't write that myself.  Essentially it's a BSD licensed community project, so I'm not sure what difference it makes (legally there is no "Xith3D Project Group" entity that I know of).  Do you have a specific concern?

It worries me a bit.  If that copyright statement is going to be included in the source code file, I would be comforted by an explicit statement that the "Xith3D Project Group" includes all contributors, including myself.  This is to distinguish it from the alternative where all contributed code becomes the sole copyright of the originators.

OK.  After duplicateNode what else is there to do for this feature?

Not real sure yet.  It has been 2 or 3 weeks since I have looked at it.

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #26 - Posted 2005-12-11 02:10:41 »

Croft,

I go by the policy that anyone who contributes any code whatsoever to a class gets an @author tag attribution.  It cuts both ways in that it gives credit where credit is due and, in the case of less stellar work, does not assign blame to the original authors when the actual change might have been made by an unattributed individual.  I will even go so far as to put an @author tag for "Unattributed" when an author is unknown or anonymous.  Here is what I wrote about it a few years ago:

"All developers who modify source code, for any reason whatsoever, should place their javadoc "@author" tag in the file and update the version date. This both alerts the other developers that changes in behavior may be due to modifications by other developers, provides credit where credit is due, and allows the code owner to track down those with experience and knowledge with certain code sections that may need debugging or enhancement. The latter is especially important with legacy source code and projects with high developer turnover."
http://croftsoft.com/library/tutorials/version/

I respect your coding practices and can see your reasoning behind them.

Xith3D however has a different policy on this matter.  The @author tag is used to indicate major authors of a particular class.  If you see someone's name on the class it means they have done some major work on it (or indeed created it).   Java.net provides us with an excellent set of CVS tools including a web based view and mailing list.  Both can be used to determine what changes have been made - the web view is good for specific classes and the mailing list good for specific users (and to view the commits in a timeline).  As far as giving credit goes, contributors (not just to Xith3D but to the Xith-TK as well) are listed on the Xith3D Credits page and I shall certianly add your name ot the list.

I believe that the most imporant thing about standards and practices is that whatever is chosen for a project, it must be applied consistantly otherwise there is no point having any at all.  I am really greatful for your contributions to Xith3D and I am hoping that you will make further ones and do so following the standards that are in place for the project (NB. that I don't mind what standards you use for a particular package of the Xith-TK, essentially those packages are "mini-projects" with their own standards, etc).


Quote
As for your last question, I'm not really sure as I didn't write that myself.  Essentially it's a BSD licensed community project, so I'm not sure what difference it makes (legally there is no "Xith3D Project Group" entity that I know of).  Do you have a specific concern?

It worries me a bit.  If that copyright statement is going to be included in the source code file, I would be comforted by an explicit statement that the "Xith3D Project Group" includes all contributors, including myself.  This is to distinguish it from the alternative where all contributed code becomes the sole copyright of the originators.

"All contributions are copyright their respective authors" is probably how I would word it.  I'm not a laywer but my understanding is that unless you sign a statment that transfers your copyright to someone else what you do belongs to you anyway.  So as far as that goes, most of Xith3D was created by Scott Shaver and David Yazel, with Yuri and myself being the biggest contributors since then.  Just as for Linux, the copyright belongs to the invidividual authors of each patch, but at the end of the day it doens't matter because it has an open source license which allows people to use it.

Best Regards,

Will.

Offline croft

Junior Member




Java, Java, Java


« Reply #27 - Posted 2005-12-12 21:54:16 »

Xith3D however has a different policy on this matter.  The @author tag is used to indicate major authors of a particular class.  If you

How would I go about getting that policy changed?  I certainly feel that a new policy would encourage contributions and therefore the growth and usefulness of Xith.

David Wallace Croft / www.CroftSoft.com / (214) 636-3790 m / Advanced Java Game Programming
Offline William Denniss

JGO Coder


Projects: 2


Fire at will


« Reply #28 - Posted 2005-12-17 01:33:09 »

You are welcome to create a thread on the topic to ask the community.  Furthermore if you want method level attributions, feel free to put an @author tag on the method javadoc.  As you can tell, I would rather just leave the standards as they are and just concentrate on other areas which need improvement more urgently Smiley

Will.

Offline Amos Wenger

Senior Member




Everything's possible, but not everything's fun...


« Reply #29 - Posted 2005-12-17 10:49:37 »

Yes, but potentials contributors may be pleased to see their names in a javadoc, even if they've just done a 2-lines bug fix that increase performance of 5%.. (which is great.. but maybe a bit exaggerated..)

"Once you start working on something, don't be afraid of failure and don't abandon it. People who work sincerely are the happiest"
Pages: [1] 2
  ignore  |  Print  
 
 
You cannot reply to this message, because it is very, very old.

 

Add your game by posting it in the WIP section,
or publish it in Showcase.

The first screenshot will be displayed as a thumbnail.

CogWheelz (18 views)
2014-07-30 21:08:39

Riven (23 views)
2014-07-29 18:09:19

Riven (15 views)
2014-07-29 18:08:52

Dwinin (12 views)
2014-07-29 10:59:34

E.R. Fleming (33 views)
2014-07-29 03:07:13

E.R. Fleming (12 views)
2014-07-29 03:06:25

pw (43 views)
2014-07-24 01:59:36

Riven (43 views)
2014-07-23 21:16:32

Riven (30 views)
2014-07-23 21:07:15

Riven (31 views)
2014-07-23 20:56:16
List of Learning Resources
by SilverTiger
2014-07-31 18:29:50

List of Learning Resources
by SilverTiger
2014-07-31 18:26:06

List of Learning Resources
by SilverTiger
2014-07-31 13:54:12

HotSpot Options
by dleskov
2014-07-08 03:59:08

Java and Game Development Tutorials
by SwordsMiner
2014-06-14 00:58:24

Java and Game Development Tutorials
by SwordsMiner
2014-06-14 00:47:22

How do I start Java Game Development?
by ra4king
2014-05-17 11:13:37

HotSpot Options
by Roquen
2014-05-15 09:59:54
java-gaming.org is not responsible for the content posted by its members, including references to external websites, and other references that may or may not have a relation with our primarily gaming and game production oriented community. inquiries and complaints can be sent via email to the info‑account of the company managing the website of java‑gaming.org
Powered by MySQL Powered by PHP Powered by SMF 1.1.18 | SMF © 2013, Simple Machines | Managed by Enhanced Four Valid XHTML 1.0! Valid CSS!