New contributor wants to submit small usability tweaks

Development
1

Hi, I’m an experienced Java and OSS developer, but I’m just getting my feet wet with game code and being OSS Java, JME seems right up my alley. I’m still in the beginning/tutorial stages, but I have noted one spot so far where I would request/recommend an addition to make the API friendlier, namely DirectionalLight currently offers only a default constructor and defaults its sole property, direction, so if one wants to create a DirectionalLight using a different direction it is necessary to use:

DirectionalLight directionalLight = new DirectionalLight();
directionalLight.setDirection(new Vector3f(-0.1f, -0.7f, -1f));

It would seem a comfortable enhancement to add:

public DirectionalLight(Vector3f direction) {
    setDirection(direction);
}

public DirectionalLight() {
    this(new Vector3f(0f, -1f, 0f));
}

Existing code should keep working as always, while new code can declare the DirectionalLight inline:

new DirectionalLight(new Vector3f(-0.1f, -0.7f, -1f))

Per the contributing instructions I am raising the matter for discussion before I submit any pull requests, but I’d like for this thread to serve as an example, proactively covering any similar, non-invasive suggestions I may make. :smile:

Thanks,
Matt

3 Likes
2

One note: DirectionalLight also inherits from Light which has a color property that might be nice to expose through a constructor also.

Edit: and I’ll add my two cents regarding the change… I think it’s a good idea. The lack of a decent constructor has annoyed me for a long time… but not enough to fix it. :slight_smile:

3

I just figured that the work doing the pull request would be more work than the little change itself and it would probably best be done when when someone is taking a larger look at things.

Maybe something just to throw in with 3.1 though I’ve heard there will be a lighting difference.

4

I completely agree… I was annoyed by this the other day and almost made the change…but you know sidetrack are evil.
+1 for this change.

More generally the 4 types of Lights should have constructors with all their vital parameters.

5

Yeah, though there is something to be said for going through the process once, I guess… and a simple change is easy to get approved, too.

On our end, a PR like this is really easy to approve if done properly.