Hopefully helpful update to the OBJLoader

Hi guys,



One of my pet hates with the OBJLoader is the rubbish naming convention used for each mesh it imports. I’ve made a simple update that grabs the name from any “o XXXX” line in the .obj file. Happy for scrutiny.



One thing that strikes me as a possible issue is if an object group has the same name. I’m not sure if this is possible to be honest. I’ve noticed that you can’t have duplicate mesh names in Blender, but maybe other 3d engines “might” do this?



Line 286-288: New method to read the “o” (object group) type.

Line 359-361: Handler condition for “o” type.

Line 401: Change of geometry name to the object group name.



[java]

/*

  • Copyright © 2009-2010 jMonkeyEngine
  • All rights reserved.

    *
  • Redistribution and use in source and binary forms, with or without
  • modification, are permitted provided that the following conditions are
  • met:

    *
    • Redistributions of source code must retain the above copyright
  • notice, this list of conditions and the following disclaimer.

    *
    • Redistributions in binary form must reproduce the above copyright
  • notice, this list of conditions and the following disclaimer in the
  • documentation and/or other materials provided with the distribution.

    *
    • Neither the name of ‘jMonkeyEngine’ nor the names of its contributors
  • may be used to endorse or promote products derived from this software
  • without specific prior written permission.

    *
  • THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
  • "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
  • TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
  • PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
  • CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
  • EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
  • PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
  • PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
  • LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
  • NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  • SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

    /



    package com.jme3.scene.plugins;



    import com.jme3.asset.
    ;

    import com.jme3.material.Material;

    import com.jme3.material.MaterialList;

    import com.jme3.math.Vector2f;

    import com.jme3.math.Vector3f;

    import com.jme3.renderer.queue.RenderQueue.Bucket;

    import com.jme3.scene.Mesh.Mode;

    import com.jme3.scene.;

    import com.jme3.scene.VertexBuffer.Type;

    import com.jme3.scene.mesh.IndexBuffer;

    import com.jme3.scene.mesh.IndexIntBuffer;

    import com.jme3.scene.mesh.IndexShortBuffer;

    import com.jme3.util.BufferUtils;

    import com.jme3.util.IntMap;

    import java.io.File;

    import java.io.IOException;

    import java.io.InputStream;

    import java.nio.FloatBuffer;

    import java.nio.IntBuffer;

    import java.nio.ShortBuffer;

    import java.util.Map.Entry;

    import java.util.
    ;

    import java.util.logging.Level;

    import java.util.logging.Logger;



    /**
  • Reads OBJ format models.

    */

    public final class OBJLoader implements AssetLoader {



    private static final Logger logger = Logger.getLogger(OBJLoader.class.getName());



    protected final ArrayList<Vector3f> verts = new ArrayList<Vector3f>();

    protected final ArrayList<Vector2f> texCoords = new ArrayList<Vector2f>();

    protected final ArrayList<Vector3f> norms = new ArrayList<Vector3f>();



    protected final ArrayList<Face> faces = new ArrayList<Face>();

    protected final HashMap<String, ArrayList<Face>> matFaces = new HashMap<String, ArrayList<Face>>();



    protected String currentMatName;

    protected String currentObjectName;



    protected final HashMap<Vertex, Integer> vertIndexMap = new HashMap<Vertex, Integer>(100);

    protected final IntMap<Vertex> indexVertMap = new IntMap<Vertex>(100);

    protected int curIndex = 0;

    protected int objectIndex = 0;

    protected int geomIndex = 0;



    protected Scanner scan;

    protected ModelKey key;

    protected AssetManager assetManager;

    protected MaterialList matList;



    protected String objName;

    protected Node objNode;



    protected static class Vertex {



    Vector3f v;

    Vector2f vt;

    Vector3f vn;

    int index;



    @Override

    public boolean equals(Object obj) {

    if (obj == null) {

    return false;

    }

    if (getClass() != obj.getClass()) {

    return false;

    }

    final Vertex other = (Vertex) obj;

    if (this.v != other.v && (this.v == null || !this.v.equals(other.v))) {

    return false;

    }

    if (this.vt != other.vt && (this.vt == null || !this.vt.equals(other.vt))) {

    return false;

    }

    if (this.vn != other.vn && (this.vn == null || !this.vn.equals(other.vn))) {

    return false;

    }

    return true;

    }



    @Override

    public int hashCode() {

    int hash = 5;

    hash = 53 * hash + (this.v != null ? this.v.hashCode() : 0);

    hash = 53 * hash + (this.vt != null ? this.vt.hashCode() : 0);

    hash = 53 * hash + (this.vn != null ? this.vn.hashCode() : 0);

    return hash;

    }

    }



    protected static class Face {

    Vertex[] verticies;

    }



    protected class ObjectGroup {



    final String objectName;



    public ObjectGroup(String objectName){

    this.objectName = objectName;

    }



    public Spatial createGeometry(){

    Node groupNode = new Node(objectName);



    // if (matFaces.size() > 0){

    // for (Entry<String, ArrayList<Face>> entry : matFaces.entrySet()){

    // ArrayList<Face> materialFaces = entry.getValue();

    // if (materialFaces.size() > 0){

    // Geometry geom = createGeometry(materialFaces, entry.getKey());

    // objNode.attachChild(geom);

    // }

    // }

    // }else if (faces.size() > 0){

    // // generate final geometry

    // Geometry geom = createGeometry(faces, null);

    // objNode.attachChild(geom);

    // }



    return groupNode;

    }

    }



    public void reset(){

    verts.clear();

    texCoords.clear();

    norms.clear();

    faces.clear();

    matFaces.clear();



    vertIndexMap.clear();

    indexVertMap.clear();



    currentMatName = null;

    matList = null;

    curIndex = 0;

    geomIndex = 0;

    scan = null;

    }



    protected void findVertexIndex(Vertex vert){

    Integer index = vertIndexMap.get(vert);

    if (index != null){

    vert.index = index.intValue();

    }else{

    vert.index = curIndex++;

    vertIndexMap.put(vert, vert.index);

    indexVertMap.put(vert.index, vert);

    }

    }



    protected Face[] quadToTriangle(Face f){

    assert f.verticies.length == 4;



    Face[] t = new Face[]{ new Face(), new Face() };

    t[0].verticies = new Vertex[3];

    t[1].verticies = new Vertex[3];



    Vertex v0 = f.verticies[0];

    Vertex v1 = f.verticies[1];

    Vertex v2 = f.verticies[2];

    Vertex v3 = f.verticies[3];



    // find the pair of verticies that is closest to each over

    // v0 and v2

    // OR

    // v1 and v3

    float d1 = v0.v.distanceSquared(v2.v);

    float d2 = v1.v.distanceSquared(v3.v);

    if (d1 < d2){

    // put an edge in v0, v2

    t[0].verticies[0] = v0;

    t[0].verticies[1] = v1;

    t[0].verticies[2] = v3;



    t[1].verticies[0] = v1;

    t[1].verticies[1] = v2;

    t[1].verticies[2] = v3;

    }else{

    // put an edge in v1, v3

    t[0].verticies[0] = v0;

    t[0].verticies[1] = v1;

    t[0].verticies[2] = v2;



    t[1].verticies[0] = v0;

    t[1].verticies[1] = v2;

    t[1].verticies[2] = v3;

    }



    return t;

    }



    private ArrayList<Vertex> vertList = new ArrayList<Vertex>();



    protected void readFace(){

    Face f = new Face();

    vertList.clear();



    String line = scan.nextLine().trim();

    String[] verticies = line.split("\s");

    for (String vertex : verticies){

    int v = 0;

    int vt = 0;

    int vn = 0;



    String[] split = vertex.split("/");

    if (split.length == 1){

    v = Integer.parseInt(split[0].trim());

    }else if (split.length == 2){

    v = Integer.parseInt(split[0].trim());

    vt = Integer.parseInt(split[1].trim());

    }else if (split.length == 3 && !split[1].equals("")){

    v = Integer.parseInt(split[0].trim());

    vt = Integer.parseInt(split[1].trim());

    vn = Integer.parseInt(split[2].trim());

    }else if (split.length == 3){

    v = Integer.parseInt(split[0].trim());

    vn = Integer.parseInt(split[2].trim());

    }



    Vertex vx = new Vertex();

    vx.v = verts.get(v - 1);



    if (vt > 0)

    vx.vt = texCoords.get(vt - 1);



    if (vn > 0)

    vx.vn = norms.get(vn - 1);



    vertList.add(vx);

    }



    if (vertList.size() > 4 || vertList.size() <= 2)

    logger.warning(“Edge or polygon detected in OBJ. Ignored.”);



    f.verticies = new Vertex[vertList.size()];

    for (int i = 0; i < vertList.size(); i++){

    f.verticies = vertList.get(i);

    }



    if (matList != null && matFaces.containsKey(currentMatName)){

    matFaces.get(currentMatName).add(f);

    }else{

    faces.add(f); // faces that belong to the default material

    }

    }



    protected String readObjectGroup() {

    return scan.next();

    }



    protected Vector3f readVector3(){

    Vector3f v = new Vector3f();



    v.set(Float.parseFloat(scan.next()),

    Float.parseFloat(scan.next()),

    Float.parseFloat(scan.next()));



    return v;

    }



    protected Vector2f readVector2(){

    Vector2f v = new Vector2f();



    String line = scan.nextLine().trim();

    String[] split = line.split("\s");

    v.setX( Float.parseFloat(split[0].trim()) );

    v.setY( Float.parseFloat(split[1].trim()) );



    // v.setX(scan.nextFloat());

    // if (scan.hasNextFloat()){

    // v.setY(scan.nextFloat());

    // if (scan.hasNextFloat()){

    // scan.nextFloat(); // ignore

    // }

    // }



    return v;

    }



    protected void loadMtlLib(String name) throws IOException{

    if (!name.toLowerCase().endsWith(".mtl"))

    throw new IOException(“Expected .mtl file! Got: " + name);



    // NOTE: Cut off any relative/absolute paths

    name = new File(name).getName();

    AssetKey mtlKey = new AssetKey(key.getFolder() + name);

    try {

    matList = (MaterialList) assetManager.loadAsset(mtlKey);

    } catch (AssetNotFoundException ex){

    logger.log(Level.WARNING, “Cannot locate {0} for model {1}”, new Object[]{name, key});

    }



    if (matList != null){

    // create face lists for every material

    for (String matName : matList.keySet()){

    matFaces.put(matName, new ArrayList<Face>());

    }

    }

    }



    protected boolean nextStatement(){

    try {

    scan.skip(”.*r{0,1}n");

    return true;

    } catch (NoSuchElementException ex){

    // EOF

    return false;

    }

    }



    protected boolean readLine() throws IOException{

    if (!scan.hasNext()){

    return false;

    }



    String cmd = scan.next();

    if (cmd.startsWith("#")){

    // skip entire comment until next line

    return nextStatement();

    }else if (cmd.equals(“o”)){

    currentObjectName = readObjectGroup();

    }else if (cmd.equals(“v”)){

    // vertex position

    verts.add(readVector3());

    }else if (cmd.equals(“vn”)){

    // vertex normal

    norms.add(readVector3());

    }else if (cmd.equals(“vt”)){

    // texture coordinate

    texCoords.add(readVector2());

    }else if (cmd.equals(“f”)){

    // face, can be triangle, quad, or polygon (unsupported)

    readFace();

    }else if (cmd.equals(“usemtl”)){

    // use material from MTL lib for the following faces

    currentMatName = scan.next();

    // if (!matList.containsKey(currentMatName))

    // throw new IOException(“Cannot locate material " + currentMatName + " in MTL file!”);



    }else if (cmd.equals(“mtllib”)){

    // specify MTL lib to use for this OBJ file

    String mtllib = scan.nextLine().trim();

    loadMtlLib(mtllib);

    }else if (cmd.equals(“s”) || cmd.equals(“g”)){

    return nextStatement();

    }else{

    // skip entire command until next line

    logger.log(Level.WARNING, “Unknown statement in OBJ! {0}”, cmd);

    return nextStatement();

    }



    return true;

    }



    protected Geometry createGeometry(ArrayList<Face> faceList, String matName) throws IOException{

    if (faceList.isEmpty())

    throw new IOException(“No geometry data to generate mesh”);



    // Create mesh from the faces

    Mesh mesh = constructMesh(faceList);



    Geometry geom = new Geometry(currentObjectName);



    Material material = null;

    if (matName != null && matList != null){

    // Get material from material list

    material = matList.get(matName);

    }

    if (material == null){

    // create default material

    material = new Material(assetManager, “Common/MatDefs/Light/Lighting.j3md”);

    material.setFloat(“Shininess”, 64);

    }

    geom.setMaterial(material);

    if (material.isTransparent())

    geom.setQueueBucket(Bucket.Transparent);

    else

    geom.setQueueBucket(Bucket.Opaque);



    if (material.getMaterialDef().getName().contains(“Lighting”)

    && mesh.getFloatBuffer(Type.Normal) == null){

    logger.log(Level.WARNING, "OBJ mesh {0} doesn’t contain normals! "
  • "It might not display correctly", geom.getName());

    }



    return geom;

    }



    protected Mesh constructMesh(ArrayList<Face> faceList){

    Mesh m = new Mesh();

    m.setMode(Mode.Triangles);



    boolean hasTexCoord = false;

    boolean hasNormals = false;



    ArrayList<Face> newFaces = new ArrayList<Face>(faceList.size());

    for (int i = 0; i < faceList.size(); i++){

    Face f = faceList.get(i);



    for (Vertex v : f.verticies){

    findVertexIndex(v);



    if (!hasTexCoord && v.vt != null)

    hasTexCoord = true;

    if (!hasNormals && v.vn != null)

    hasNormals = true;

    }



    if (f.verticies.length == 4){

    Face[] t = quadToTriangle(f);

    newFaces.add(t[0]);

    newFaces.add(t[1]);

    }else{

    newFaces.add(f);

    }

    }



    FloatBuffer posBuf = BufferUtils.createFloatBuffer(vertIndexMap.size() * 3);

    FloatBuffer normBuf = null;

    FloatBuffer tcBuf = null;



    if (hasNormals){

    normBuf = BufferUtils.createFloatBuffer(vertIndexMap.size() * 3);

    m.setBuffer(VertexBuffer.Type.Normal, 3, normBuf);

    }

    if (hasTexCoord){

    tcBuf = BufferUtils.createFloatBuffer(vertIndexMap.size() * 2);

    m.setBuffer(VertexBuffer.Type.TexCoord, 2, tcBuf);

    }



    IndexBuffer indexBuf = null;

    if (vertIndexMap.size() >= 65536){

    // too many verticies: use intbuffer instead of shortbuffer

    IntBuffer ib = BufferUtils.createIntBuffer(newFaces.size() * 3);

    m.setBuffer(VertexBuffer.Type.Index, 3, ib);

    indexBuf = new IndexIntBuffer(ib);

    }else{

    ShortBuffer sb = BufferUtils.createShortBuffer(newFaces.size() * 3);

    m.setBuffer(VertexBuffer.Type.Index, 3, sb);

    indexBuf = new IndexShortBuffer(sb);

    }



    int numFaces = newFaces.size();

    for (int i = 0; i < numFaces; i++){

    Face f = newFaces.get(i);

    if (f.verticies.length != 3)

    continue;



    Vertex v0 = f.verticies[0];

    Vertex v1 = f.verticies[1];

    Vertex v2 = f.verticies[2];



    posBuf.position(v0.index * 3);

    posBuf.put(v0.v.x).put(v0.v.y).put(v0.v.z);

    posBuf.position(v1.index * 3);

    posBuf.put(v1.v.x).put(v1.v.y).put(v1.v.z);

    posBuf.position(v2.index * 3);

    posBuf.put(v2.v.x).put(v2.v.y).put(v2.v.z);



    if (normBuf != null){

    if (v0.vn != null){

    normBuf.position(v0.index * 3);

    normBuf.put(v0.vn.x).put(v0.vn.y).put(v0.vn.z);

    normBuf.position(v1.index * 3);

    normBuf.put(v1.vn.x).put(v1.vn.y).put(v1.vn.z);

    normBuf.position(v2.index * 3);

    normBuf.put(v2.vn.x).put(v2.vn.y).put(v2.vn.z);

    }

    }



    if (tcBuf != null){

    if (v0.vt != null){

    tcBuf.position(v0.index * 2);

    tcBuf.put(v0.vt.x).put(v0.vt.y);

    tcBuf.position(v1.index * 2);

    tcBuf.put(v1.vt.x).put(v1.vt.y);

    tcBuf.position(v2.index * 2);

    tcBuf.put(v2.vt.x).put(v2.vt.y);

    }

    }



    int index = i * 3; // current face * 3 = current index

    indexBuf.put(index, v0.index);

    indexBuf.put(index+1, v1.index);

    indexBuf.put(index+2, v2.index);

    }



    m.setBuffer(VertexBuffer.Type.Position, 3, posBuf);

    // index buffer and others were set on creation



    m.setStatic();

    m.updateBound();

    m.updateCounts();

    //m.setInterleaved();



    // clear data generated face statements

    // to prepare for next mesh

    vertIndexMap.clear();

    indexVertMap.clear();

    curIndex = 0;



    return m;

    }



    @SuppressWarnings("empty-statement")

    public Object load(AssetInfo info) throws IOException{

    reset();



    key = (ModelKey) info.getKey();

    assetManager = info.getManager();

    objName = key.getName();



    String folderName = key.getFolder();

    String ext = key.getExtension();

    objName = objName.substring(0, objName.length() - ext.length() - 1);

    if (folderName != null && folderName.length() > 0){

    objName = objName.substring(folderName.length());

    }



    objNode = new Node(objName + "-objnode");



    if (!(info.getKey() instanceof ModelKey))

    throw new IllegalArgumentException("Model assets must be loaded using a ModelKey");



    InputStream in = null;

    try {

    in = info.openStream();



    scan = new Scanner(in);

    scan.useLocale(Locale.US);



    while (readLine());

    } finally {

    if (in != null){

    in.close();

    }

    }



    if (matFaces.size() > 0){

    for (Entry<String, ArrayList<Face>> entry : matFaces.entrySet()){

    ArrayList<Face> materialFaces = entry.getValue();

    if (materialFaces.size() > 0){

    Geometry geom = createGeometry(materialFaces, entry.getKey());

    objNode.attachChild(geom);

    }

    }

    }else if (faces.size() > 0){

    // generate final geometry

    Geometry geom = createGeometry(faces, null);

    objNode.attachChild(geom);

    }



    if (objNode.getQuantity() == 1)

    // only 1 geometry, so no need to send node

    return objNode.getChild(0);

    else

    return objNode;

    }



    }



    [/java]

OK, so I don’t have a straightforward “diff” tool that outputs this on the command-line. I have updated the topic with the three changes I’ve made.



Does that help?

Why not submit a diff instead? Would be much easier.

If you use TortoiseSVN to download the source code then change those files the right click menu has a “create patch” option. Do that and it gives you the changes in patch format that you can paste into here and people can then compare and apply.

(I imagine other SVN clients have similar functionality, tortoise is just the one I’m used to).

@richard_hawkes said:
OK, so I don't have a straightforward "diff" tool that outputs this on the command-line.

The jme SDK comes with a diff tool.

Ho hum, are my line comments in the topic not sufficient?



I’m not going to install TortoiseSVN. I had some issues with performance last time I used it!



But trusty “Beyond Compare” has come to my rescue… Is this what you boys are after?



[java]

Left file: C:jme3sourcecomjme3scenepluginsOBJLoader.java

Right file: H:AppsXpDesktopOBJLoader.java

284a286,288

> protected String readObjectGroup() {

> return scan.next();

> }

354a359,360

> }else if (cmd.equals(“o”)){

> currentObjectName = readObjectGroup();

395c401

< Geometry geom = new Geometry(objName + “-geom-” + (geomIndex++), mesh);



> Geometry geom = new Geometry(currentObjectName);

[/java]

Almost, doesn’t look like diff/patch format to me though. Posting the whole class or just a list of lines is pretty much assuring your addition won’t even be reviewed, yeah.

Edit: Do you code in wordpad? I don’t know any IDE that has no diff tools…?

@normen said:
Do you code in wordpad? I don't know any IDE that has no diff tools..?

I code in NetBeans, and the diff tool there is graphical. Do you want me to print off screen dumps?

I am looking at all of the other topics here, and I don't see any clear format. Indeed, I can see code snippets as I've done above without half the hassle. Nor can I grasp this from the Contributors Handbook what it should look like.

Lol, you can export the diff via the menu when you look at it in the IDE ^^ Diff/patch is something very basic when its about contributing things or working with others on code, I guess you should get acquainted with the tools.

Well @normen, then you’re going to have to spell that out to me, because when I do that I get the full source code again.



To be honest, this was only meant to be an offering, and if it’s going to be this difficult to cut through the red tape and I only serve to get patronised, then I think I’ll just keep my changes to myself.


  • Richard
@richard_hawkes said:
Well @normen, then you're going to have to spell that out to me, because when I do that I get the full source code again.

To be honest, this was only meant to be an offering, and if it's going to be this difficult to cut through the red tape and I only serve to get patronised, then I think I'll just keep my changes to myself.

- Richard


That is your right, of course. We're not trying to be nitpicky jerks but if you submit patches in a form that we can't easily use then we have to honestly tell you that they might not get applied. We're not trying to be pedantic, just honest.

When you complain about not being able to perform a function that is pretty basic and a corner stone to 99% of professional software development then we are not being patronizing by telling you that. We are trying to help you become a better contributor and trying to help you realize that this particular function isn't some weird arcane ritual.

The amount of time any of us have to apply quality patches is vanishingly small. When it is submitted as a diff, this is a relatively pain-free process... a quick click gets it applied and we can devote all of the time to making sure it is a good update against the latest, code, etc.. When it is not submitted as a diff, you might as well have printed it out, marked it up, and scanned it in as a PDF... it's nearly the same amount of work for us to apply in that case as non-diff'ed submissions, ie: we have to do 20 times more leg work to make sure to get the code right. Then we still have to spend the time making sure it is a good update against the latest code, etc..

And this part will sound elitist and condescending from your perspective but understand that I'm just being honest about the way most more professional developers will see this conversation. Basically, if a contributor can't figure out how to do something as basic as submitting diffs then it brings to question whether that contributor is qualified to make quality patches at all or if we will have to do extra work to make them play right. In which case, our limited unpaid support time could be applied in more productive ways.

What he said, sorry if I came off harsh.

First right-click the file and select which other file to diff it to:



Then export the result, flipping input and output if necessary.

@pspeed said:
Basically, if a contributor can't figure out how to do something as basic as submitting diffs then it brings to question whether that contributor is qualified to make quality patches at all or if we will have to do extra work to make them play right.


So watch out folks. If you can't diff properly, you can't code properly :(

I should tell you I have been working in IT for 25 years both as a developer, and now an IT manager for an investment bank. In my time I've checked in plenty of code and been party to (and overseen) many a code review. And I have delivered some VERY expensive solutions! To say that this particular patch/diff approach is a 99% cornerstone of professional development is just plaing wrong IMO. It's just a method used as part of this project (and other projects I'm sure, but I hope you see my point). I don't consider myself an idiot, and I would say that this patch/diff tool is just one of the many ways to compare two files. I have actually submitted 2 different diffs. One detailing the changes (at the start of the topic), a command-line diff (similar to that in Unix). I also tried to "bold" the changes on the source code dump, but the "strong" doesn't work in "java" tags.

What I would say is that I've never worked on an open source project accross such a resource constrained environment, and I appreciate that you require a consistent and standard approach for all submissions (as would I). Therefore, I'm gonna suck it up, disregard your "am I worthy" comments, and get this diff off to you tonight. It was such a simple fix that I didn't think it would cause such consternation.

And thanks for spelling it out to me @normen, I did manage to find it in the end. I notice there are two formats. I’m taking no chances and putting both in!

Consider that the class you pasted above has over 500 lines and you’ve made 3 changes. There’s no comparison. It’s not obvious what was changed to what. Nobody says you’re a bad coder. Don’t take things personal.



The point is, put yourself in our place. Would you prefer to gouge your eyes out comparing the content of 3 lines in a 500 lines files or would you rather look at something like this:



[java]

This patch file was generated by NetBeans IDE

It uses platform neutral UTF-8 encoding and n newlines.

— Base (BASE)

+++ Locally Modified (Based On LOCAL)

@@ -9,6 +9,7 @@

import com.madjack.games.disenthral.utils.AiCommand;

import com.madjack.games.disenthral.utils.AiCommand.Command;

import com.madjack.games.disenthral.utils.GameCommands;

+import java.util.concurrent.atomic.AtomicBoolean;

import java.util.logging.Level;

import java.util.logging.Logger;



@@ -20,10 +21,10 @@

private transient SimpleApplication app;

private transient AppStateManager asm;

private final static int BUILD_STAGES = 5;

  • private static boolean letsRun = false;
  • private static boolean isGalaxyMade = false;
  • private volatile static boolean readyToBuild = false;
  • private volatile static boolean isLoading = false;
  • private boolean letsRun = false;
  • private boolean isGalaxyMade = false;
  • private boolean readyToBuild = false;
  • private AtomicBoolean isLoading = new AtomicBoolean(false);

    protected GameLogic processor;

    private volatile GameCommands.Command gameCmd = GameCommands.Command.Nothing;

    private AiCommand.Command aiCmd = Command.Nothing;

    @@ -49,9 +50,7 @@

    waitHere(0);

    } else {

    if (gameCmd == GameCommands.Command.LoadGame) {
  •                synchronized (this) {<br />
    
  •                    isLoading = true;<br />
    
  •                }<br />
    
  •                isLoading.set(true);<br />
    

createLeafManager();

if (!loadSavedGame()) {

throw new UnknownError("Could not LOAD saved game.");

@@ -74,9 +73,7 @@

asm.getState(GameState.class).setIsMenuOpen(false);

asm.getState(GameState.class).setIsPickingAllowed(true);

asm.getState(GameState.class).setIsGameRunning(true);

  •                    synchronized (this) {<br />
    
  •                        isLoading = false;<br />
    
  •                    }<br />
    
  •                    isLoading.set(false);<br />
    

System.out.println("Game loaded SUCCESSFULLY.");

}

} else if (gameCmd == GameCommands.Command.SaveGameAndExit

@@ -279,8 +276,6 @@

}



public boolean isLoading() {

  •    synchronized (this) {<br />
    
  •        return isLoading;<br />
    
  •    return isLoading.get();<br />
    

}

}

-}

[/java]

…lets say diff/patch knowledge is mandatory for open source software. For one-customer or in-house solutions I’ve seen about everything in terms of development environment now I guess, so there are no standards for that really :slight_smile: Linux for example could not live without diff/patch and you cannot contribute to core Linux projects w/o it.



Edit: aaah, thats something that instantly talks much more than a 3-page post with a whole class in it @madjack

@normen said:
Edit: aaah, thats something that instantly talks much more than a 3-page post with a whole class in it @madjack


Exactly. It's so easy to compare an old behavior with the diff code. At a glance you can see "X did this, now it does that". It's simple, direct and obvious. It's just incomparable, literally, with a pasting of an entire class.

Anyway, I think our point got across. And please @richard_hawkes don't take it personally and don't think we're doing this to belittle you or your abilities. Others might stumble on this post and better understand why diff is much better for everyone involved. :)
@richard_hawkes said:
What I would say is that I've never worked on an open source project accross such a resource constrained environment, and I appreciate that you require a consistent and standard approach for all submissions (as would I). Therefore, I'm gonna suck it up, disregard your "am I worthy" comments, and get this diff off to you tonight. It was such a simple fix that I didn't think it would cause such consternation.


It wasn't a personal attack. I don't know you and the only experience I had to go on was that you had trouble creating a patch and then got upset because of some pushback. I was just sharing what opinions people form when they have no other facts to go on.

I've worked on plenty of open source projects and pretty much all of them require patches for contributions (at least all of the ones that don't hand out committer status to anyone who requests it). Most of the apache/jakarta projects I worked on would outright reject anything not a patch... at least back when I followed them directly.

And they all look strangely at people that can't figure out "svn diff > my-changes.patch" or the IDE equivalent.

Now I will admit that one project (I think it was open scene graph) required full files because that developer got burned so many times from people submitting diffs against old source, I guess. Diff submission is so common on open source projects that he had to remind contributors all the time because they were constantly submitting diffs.