Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Add documentation for Util class #5

Open
wants to merge 1 commit into
base: 1.16.x
Choose a base branch
from

Conversation

sciwhiz12
Copy link
Contributor

Adds documentation for the net.minecraft.util.Util class.

Notes:

  • I deliberately avoided the use of \ for the Doxygen commands, for two reasons:
    1. The @ character is more easily distinguishable from the surrounding text than \.
    2. It may make it semi-easier if the documentation would be ported to proper Javadoc comments (such as if MMMS is fully operational.
  • Some methods' complex generics left them unable to be parsed when using their whole method signature in the @fn structural comment. Doxygen does fine without the generics, however the detailed section for that method will have the generics missing. To
    To mitigate this, I also pasted the complete method signature of the methods into their description. (Any advice on how to fix the problem better would be appreciated.)
  • It is a deliberate choice to indent the methods/fields of inner classes/enums, to distinguish them from the methods/fields of the outermost class.
  • One method is left undocumented: Util.attemptDataFix. For the sake of preserving my sanity minimizing the total time that I invest in this singular class, I will leave documenting it to future contributors.

@sciwhiz12 sciwhiz12 added the documentation This is an improvement or addition to the documentation. label Nov 26, 2020
@sciwhiz12 sciwhiz12 requested a review from a team November 26, 2020 11:59
Copy link

@ChampionAsh5357 ChampionAsh5357 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a quick side note, we are expecting a new mappings version to release before the end of the year. So, I would consider holding off on merging until then as some of the parameter and method names might be mapped.

Comment on lines +252 to +253
* This is used to initialize collections in static fields with their contents (without using builders such as
* @ref ImmutableMap.Builder).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a good example to include for this method. If an immutable map was to be created, it would still need to go through builder to input the corresponding elements. Otherwise the method itself would need to be wrapped within the map's copyOf method or called with Buiilder#build. This is why the above method exists with the supplier to allow access to the builder and constructing within one method without an external invocation.

*/

/**
* @fn public static <T> Optional<T> Util::acceptOrElse(Optional<T> opt, Consumer<T> consumer, Runnable orElse)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For those more familiar with Java 9+, I would suggest mentioning its equivalent Optional#ifPresentOrElse.

/**
* @fn public static void Util::backupThenUpdate(File current, File latest, File oldBackup)
*
* Backups the first file to the third file, then updates the first file from the second file.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should use the parameter names instead of the parameter order.

* Returns a @ref DataResult with the integers from the stream, depending on the amount of remaining elements.
*
* This method returns a @ref DataResult with an array of integers from the stream if it contains at least @p size integers;
* else it will return a failed @ref DataResult, which may contain the remaining integers in the stream, if any.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the length of the array is not larger than or equal to the specified size, no partial result will be given (meaning the array will not be passed in), only the error message.

/**
* @fn public static void Util::func_240984_a_(Path p_240984_0_, Path p_240984_1_, Path p_240984_2_)
*
* Copies the source path from the parent path to the destination path.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p_240984_0_ can be considered to be the root directory in this case. p_240984_2_ is the subdirectory within p_240984_0_ to which the relative path is constructed. Then, the resulting relative path is applied to p_240984_1_, where the files from p_240984_2_ are copied towards. It would be more appropriate to call them the original root, new root, and resolved path.

As an example, if we had the following arguments a/b, e/f, and a/b/c/d, path would grab the following relative c/d/ and apply it to e/f resulting in e/f/c/d. The file is then copied from a/b/c/d to e/f/c/d.

Copy link
Contributor

@dpeter99 dpeter99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some nitpicking. But most of the stuff seamed good.
I leave it up to you if you want to merge it in now and fix up the stuff for the new mapping or wait.

*
* The default supplier returns the value of @ref System#nanoTime().
* The value is not related to system or wall-clock time. It only measures the time in nanoseconds since some arbitrary
* "origin" time. All invocations of this method within a JVM instance will return from the same time-source and origin time.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a "for getting the time since a know point use millisecondsSinceEpoch()"

*/

/**
* @fn Util::getValueName(Property<T> property, Object value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ones seams to be missing the return type in the signature

*
* Creates a translation key for the given nullable @ref ResourceLocation based on the given type.
*
* If the @p id parameter is @c null, then the returned translation key will be <tt>type + ".unregistered_sadface"</tt>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing it works with any type not just the ones that actually exist. But let's spell it out I think.

* Returns the current value of the JVM's time source in milliseconds.
* This is equivalent to calling <tt>Util.@ref nanoTime() / 1000000L</tt>
*
* @return The current value of the JVM time source in milliseconds.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See Util::nanoTimeSupplier
And smae for the one below

/**
* @fn public static Type<?> Util::attemptDataFix(TypeReference type, String choiceName)
*
* Attempts to create a data fixer for the given @ref TypeReference.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessign this would link to the data fixer docs if that would exist....

*/

/**
* @fn public static <K> Strategy<K> Util::identityHashStrategy()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seams to have a wrong signature.

*/

/**
* @fn Util::gather(List<? extends CompletableFuture<? extends V>> futuresIn)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No return.

* Returns the given @ref Runnable with the given supplied name.
*
* The implementation of this simply returns the passed in runnable directly.
* However, this method if edited by developers can be changed to wrap the runnable in another runnable before returning,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this method be edited... Feels wired to suggest it, and sound like core modding.....

*/

/**
* @fn public static <T extends Throwable> T Util::pauseDevMode(T throwableIn)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could add soemthing for the methods that are Mojang used and we don't really know what they are for.....

* Returns a random object from the given array, based on the given @ref Random.
*
* The randomness of the selection shall conform to the characteristics of the given @ref Random, based on the
* behavior of @ref Random#nextInt(int).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean it uses the Random#nextInt(int) ? Or somehting more complicated?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation This is an improvement or addition to the documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants