Adding the newdawn loaders to the distribution

I am not. I am just using some MD2 files I have from before. Why do you ask?

MD2Loader2 works.

Mainly because I want to start adding animations to my game and wondered if you used editor X version Y for compatibility assurance

Can you please hold off overwriting the old ones for a few days, I havn’t had a chance to check it out yet. :slight_smile:

Thanks,

Will.

OK, let me know when ready.

croft,

I have had a chance to look at your changes. They look great to me. I think your enhancements are very useful.

I have a few non-functionality related issues which may sound petty but I feel are important for a community project such as this one:

1. When making changes to code such as this, it’s better not to combine formatting changes with functionality changes, as the latter can get lost in the former (making it harder for someone evaluating to actually see what’s changed). I’m referring to changes like this:


-        return new MD2RenderedFrame(frame.getName(),fanArray,stripArray,fanTexArray,stripTexArray,fanCounts,stripCounts);
+        return new MD2RenderedFrame(
+          frame.getName(),
+          fanArray,
+          stripArray,
+          fanTexArray,
+          stripTexArray,
+          fanCounts,
+          stripCounts);
     }

By all means make formatting changes that help readability as they do in this example, but please keep those changes to a seperate commit.

2. Is there any reason the entire class is indented one level? Because every line is changed, diff will show the entire file being removed and replaced. While on my machine I can (and did) use the -w argument, the diff’s generated by Collabnet (which can be handy) will be useless.

3 Is there any reason why you havn’t followed the coding style of the original MD2Loader / xith3d project in general? We don’t put brackets on a new line, nor use the bulk slashes for comments. To me, MD2Loader is more readable than MD2Loader2, but stylistic preferences aside, I think it is always best to stick to the one style for a project, and at least stick to the same style for a single package-tool. If you were adding a stand-alone tool of your own creation, I wouldn’t mind, but having two different styles used in the one package I think makes it harder to understand the package as a whole.

To summarise: I think the API changes are excellent, but the formatting changes not so. Would it be possible to keep the code style as it is?

I really hope I havn’t offended you, the last thing I want to do is push away someone who is making a valuable contribution to Xith3D as you are - but I feel I need to raise these issues.

Best Regards,

Will.

Whenever I substantially revise a class written by someone else, I often reformat as I go so that it gradually becomes comprehensible to me. I learned to program in the military which means using a format that is as readable as possible to ensure maintainability. The style you see there is something that I evolved over 25 years.

I’m not willing to spend the time to revert the code style but I would have no objections if someone else did so, perhaps using an automated tool such as an IDE.

Fair enough. Do you agree with my comments though?

Comment #3 was made from a maintainability point of view as I think it is best to have one formatting style for one module. Comment #1 I do feel is important because if someone is trying to isolate a regression bug by looking back though the commit logs and at changes, it’s much easier when any stylistic changes are in their own commits (and therefore can simply be ignored by the bug hunter).

Like I said, I’m only referring to the modification of existing modules here.

Thanks again for the enhancements, if I find the time I will merge the changes and commit, until then it’s perfectly usable as it is.

Cheers,

Will.

I had looked at the Xith coding conventions earlier and had decided that Xith really did not have a standard at the moment:
http://xith.org/tiki-index.php?page=CodeConventions

I wrote a rigorous style guide myself a few years back:
http://www.croftpress.com/david/research/java/guide/

The most recent one that I wrote at a company, however, was much more lenient. I pretty much just said read Chapter 7 in Java in a Nutshell and don’t use tab characters. I will nag novice junior Java programmers about some things but I don’t put up a fuss if they ignore me. I’m pretty much a believer in code ownership.

When I’m working with code that someone else wrote, I will keep the same style if the changes are minor and I’m not inheriting the code on a permanent basis. If the changes are major such as an overhaul of most of a class or I am inheriting the code permanently, I will make the changes.

Well it pretty much does:

Only that the list of exceptions and refinements was never compiled. One difference I know of is that four spaces are used for indenting.

Will.

Hi
I’ve got this .obj File I tried to load. It contains Textures. By loading with the old OBJLoader I get the following Exception:


Exception in thread "main" java.lang.StringIndexOutOfBoundsException: String index out of range: -1
	at java.lang.String.substring(String.java:1768)
	at org.xith3d.loaders.obj.OBJLoader.load(OBJLoader.java:35)
	at org.xith3d.loaders.obj.OBJLoader.load(OBJLoader.java:31)
	at org.xith3d.loaders.obj.OBJLoader.load(OBJLoader.java:27)
	at core.GameObject.<init>(GameObject.java:72)
	at core.Map.<init>(Map.java:47)
	at core.Client.main(Client.java:171)

When loading with OBJLoader2 I get no Exception at all, but it simply returns null.

I think this is a really bad change, because without an Exception one wouldn’t know it was the loader the caused the error. People might search themselfes to death (who would be assuming that the Loader doesn’t work correctly, when it doesn’t give a mesaage)
We could omit the Stack-Trace, but then we would at least have to make a message appear.

Arne

PS: I would of course happy if someone could help me out, why this error occurs.

PPS: No I haven’t changed anything in the .obj file. It’s exactly the one blender exported.

Ok now I’ve solved this issue (with help of the Stack-Trace :wink: ), but now It still doesn’t load correctly:
OBJLoader simply returns null.
But now OBJLoader2 gives me more information (and actually also loads the File, only there are no Textures applied). It prints me these Messages:

org.xith3d.loaders.obj.MaterialLibLoader2:  ignoring unknown material tag:  "d 1.0"
org.xith3d.loaders.obj.MaterialLibLoader2:  ignoring unknown material tag:  "illum 2"
org.xith3d.loaders.obj.MaterialLibLoader2:  ignoring unknown material tag:  "d 1.0"
org.xith3d.loaders.obj.MaterialLibLoader2:  ignoring unknown material tag:  "illum 2"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.001_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.002_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.003_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.004_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.005_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.006_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.007_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.008_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.009_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.010_None"
org.xith3d.loaders.obj.OBJLoader2:  ignoring unknown OBJ tag:  "o Sphere.011_None"

At least it prints me these messages :slight_smile: Very good!!

Arne

There’s several OBJ Exporters scripts for Blender, and there’s only one that’s correct.
You can find it here http://blender3d.org/cms/Import___Export.5.0.html It’s the “Wavefront OBJ Importer/Exporter”.

I recompiled with the latest version of MD2Loader2 from arne and it crashed due to a minor bug. I fixed the bug and the changes arne made look great. Suddenly normals are working now! Shouldn’t arne add his @author tag to the MD2Loader2 javadoc?

no - don’t add me!
I only commited the code from 5parrowhawk. So add 5parrowhawk !

I just checked the cvs History (I thought I am going insane, because I didn’t know that I’ve commited something like that)

Does 5parrowhawk have commit access?

I believe he didn’t wanted to deal with the cvs stuff directly. (I’m not sure anymore) … But - yes he has commit access. (Just checked the Membership list of the xith-tk)

No he doesn’t. That’s why arne committed it for him.

I found the topic where 5parrowhawk and arne talk about committing the code:
http://www.java-gaming.org/forums/index.php?topic=11440

I sent 5parrowhawk a message asking him if it would be OK to add his @author tag to MD2Loader2.

I received no response 5parrowhawk. I added his @author tag to MD2Loader2 as I think it is the right thing to do. I think we need to add the @author tag of any contributor or at least “unascribed” to avoid plagiarism.
http://onelook.com/?w=plagiarism&ls=a
http://java.sun.com/j2se/javadoc/writingdoccomments/index.html#@author
http://java.sun.com/j2se/javadoc/writingdoccomments/index.html#multiple@author