com.jme3.anim.tween.action.BlendSpace: basic javadoc

[pavly-gerges]

This PR adds a basic javadoc for the BlendSpace class.

[stephengold]

Javadoc is important, and I appreciate the considerable effort that's gone into this PR. Unfortunately, it's still very confusing. @Scrappers-glitch are you willing to continue refining it?

[pavly-gerges]

@stephengold For me, this PR is complete, you can point to the confusing and/or missing parts, please don't forget to mention why these parts are confusing and a superior format for describing these confusing parts.

[pavly-gerges]

@stephengold In this PR, I chose to stick to describing the behavior of the pattern used, or maybe the interaction of the API components, if you want to understand more, please use this paradigm designed by Remy (Most of this documentation is based on this paradigm, the talks on forums and my own techdemos):
Monkanim: new animation system in the works

EDIT:
Here are some examples of implementing new BlendSpaces in a user application:
CustomBlendAction and CustomBlendSpaces

[pavly-gerges]

Okay I am willing to refine this. @stephengold Do you recommend specific refinements to make it more simple. Maybe code samples? or more mathematical terms? any ideas so far?

[stephengold]

Figuring out why the javadoc is confusing and writing a clear explanation seems like a lot of effort. I don't want to spend more time on this PR.

[pavly-gerges]

BlendSpace is all about a provider interface. It provides the blend weight value to blend between 2 actions. Blend weight is then used as the value of interpolation. That simple. I like to think of it from a higher perspective as an interface that enables the user to adjust the behavior of blending (a controller interface from an architectural perspective), and that blend weight value is a read-only value. It could be manipulated using the mediator BlendSpace#setValue(...).

[pavly-gerges]

For me, this PR has completed. Please let me know if there are any changes you want to add, otherwise it's ready for merging.

[pavly-gerges]

It would be great if you consider this PR on jme-3.8 if possible. It has been a year since no one has reviewed this PR and requested any changes. I guess its complete. However, if clarification is required, I will happily try to provide.

[yaRnMcDonuts]

I agree it should be merged for 3.8 if possible, I must have missed this when I was looking through PRs while searching for ones like this that that are finished or suitable for the next release.

I took a look at the changes and it looks good to me. But I know there's been some previous discussion, and I personally haven't used this class much yet myself (so any mistakes wouldn't stand out to me right away).

So I will wait 2 days in case anyone has any further review, and then I will merge.

[pavly-gerges]

I would like to hear from @stephengold; as there were some conflicts about the way things got to be explained so far. In the meantime, I will update the copyright and see if anything could be added.

[stephengold]

I would like to hear from @stephengold; as there were some conflicts about the way things got to be explained so far.

As I said a year ago, I don't want to spend any more time on this PR.

[capdevon]

I asked the AI ​​to check the javadoc for errors and rewrite it in a way that conforms to java standards. Below is the analysis of the defects identified by the AI:

• Incomplete and unclear description:
  The description of the interface and methods should be clearer and more detailed. For example, the description of BlendSpace should better explain the context and use of the interface.

• Improper use of HTML tags:
  The use of <li> tags within <p> tags is incorrect. List items should be enclosed in <ul> or <ol> tags.

• Notes and warnings:
  Notes and warnings should be more prominent and separated from the main text to improve readability.

Here is an improved version:

/**
 * A provider interface that supplies a value to control the blending between
 * two successive actions in a {@link BlendAction}. The blending weight is a
 * read-only value, adjustable using the arbitrary value set during runtime.
 * 
 * <p>
 * Blending is the process of mixing two successive animation
 * {@link BlendableAction}s by interpolating their transforms and applying the
 * result to the assigned {@link HasLocalTransform} object. The
 * {@link BlendSpace} provides this blending action with a blend weight value.
 * </p>
 * 
 * <p>
 * The blend weight is the value used for interpolation of the target transforms
 * and must be in the range [0, 1].
 * </p>
 * 
 * <p>
 * Different blending weight scenarios managed by {@link BlendAction}:
 * <ul>
 *   <li>0 < Blending weight < 1: Blending is executed each update among two actions. The first action uses a blend value of 1, and the second action uses the blend space weight for interpolation.</li>
 *   <li>Blending weight = 0: Blending hasn't started yet; only the first action is interpolated at weight = 1.</li>
 *   <li>Blending weight = 1: Blending is finished; only the second action continues to run at weight = 1.</li>
 *   <li>Negative values and values greater than 1 are not allowed (extrapolations are not allowed).</li>
 * </ul>
 * </p>
 * 
 * <p>
 * For more details, see {@link BlendAction#doInterpolate(double)} and
 * {@link BlendAction#collectTransform(HasLocalTransform, Transform, float, BlendableAction)}.
 * </p>
 * 
 * Created by Nehon.
 * 
 * @see LinearBlendSpace an example of blend space implementation
 */
public interface BlendSpace {

    /**
     * Adjusts the target blend action instance that will utilize the blend weight
     * value provided by this blend-space implementation.
     * 
     * @param action the blend action instance that will utilize this blend-space (not null).
     */
    public void setBlendAction(BlendAction action);

    /**
     * Provides the blend weight value to the assigned {@link BlendAction} instance.
     * This value will be used for interpolating a collection of actions'
     * transformations (keyframes).
     * 
     * @return the blending weight value in the range from 0 to 1. Negative values
     *         and values above 1 are not allowed.
     */
    public float getWeight();

    /**
     * Sets an arbitrary value used for adjusting the blending weight value.
     * 
     * @param value the value in floats.
     */
    public void setValue(float value);
}

Edit:
I don't have a strong opinion about it, but since this PR has already been rejected in the past by expert people, if you really want to integrate it, please consider making some improvements.

[pavly-gerges]

I appreciate your comment Mr. Wyatt @capdevon. However, sometimes the AI Models introduce polished solutions which are really annoying.

For example, what is the core difference between:

* A provider interface which provides a value {@link BlendSpace#getWeight()} to control the blending between 2 successive actions in a {@link BlendAction}.

And this:

A provider interface that supplies a value to control the blending between
 * two successive actions in a {@link BlendAction}. 

Really?

I will go over them anyway, and see if there are things to add, but core changes, I don't think so.

[yaRnMcDonuts]

I believe @pavly-gerges did a decent job addressing some of the initial concerns that were raised.

I wouldn't say that your wording was the wording I would have used in some cases, but it did seem to be understandable enough and objectively correct in your description of methods when I reviewed it. And that is the important part in my opinion.

But I also agree that the some of the suggested changes from @capdevon improve upon it even further.

I'm typically skeptical of using AI for jme related things (it seems to get things wrong about jme quite a lot), but I think for javadocs it is one place where Ai can do a goodjob to help refine the word choice and make things more understandable. I wouldn't trust AI to write javadocs all on its own, but it certainly has done a good job refining and finalizing some aspects the existing javadoc that you gave it.

[yaRnMcDonuts]

Really?

I will go over them anyway, and see if there are things to add, but core changes, I don't think so.

I think most of your wording is acceptable, there's only a few places where I feel that the AI generated changes offered some improvements.

The more I compare, the less big differences I see.

But, for example, 1 place that I do prefer the wordage of the AI generated javadoc would be here:

<li> The blend weight value must be in this interval [0, 1]. </li>..

The proposed changes swap out the word "interval" with "range", and I'd say that the word "range" is somewhat better of a choice here. "Interval" is still understandable but the word "range" is probably a better choice of the two.

[yaRnMcDonuts]

@pavly-gerges Do you have plans to make any more adjustments to this PR?

If not, then I will merge it in about 24 hours.

I do not think that any of the suggested wording changes are major enough to warrant holding up a PR for a basic javadoc any longer. Especially considering that there are language differences amongst jme users and english is not the first language for many, so the perceived "best wording" is not always going to be consistent and agreed upon.

In its current state, your javadoc is understandable and does a satisfactory job explaining the class' functionality, and I do not see any misleading or incorrect explanations of methods or variables... and ultimately it does a way better job than having no javadoc at all. And if anyone wants to improve some wording in the future, then that can always be adjusted in a future PR.

[pavly-gerges]

@pavly-gerges Do you have plans to make any more adjustments to this PR?

I am willing to do some refinements, just for the benefit of the community. Coming up today, keep updated!

[pavly-gerges]

For me, this is ready to be merged. Please note that, if you find anything that you would like to change, I have no problems with that; you can just change them directly before merging.