Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model system rework #6180

Open
wants to merge 33 commits into
base: 1.14.x
from
Open

Conversation

@gigaherz
Copy link
Collaborator

gigaherz commented Sep 22, 2019

This is the initial implementation of my new model system idea.

This feature expands the vanilla model loading with a new key: "loader". If this key is found in the model, forge will derive a model instance from any necessary values seen in the json.

Example, models/item/test_fluid_bucket.json

{
  "parent": "forge:item/bucket",
  "loader": "forge:bucket",
  "fluid": "new_fluid_test:test_fluid"
}

The "parent" key is handled by the vanilla logic, which in this case takes care of providing the required default textures for the bucket. The "forge:bucket" loader constructs a custom model that uses a base texture and some mask textures, to produce a custom bucket model for the given fluid in the "fluid" key.

I need people to test this! Here's how:

  1. Clone/checkout the forge repository git clone https://github.com/MinecraftForge/MinecraftForge.git
  2. Fetch and checkout my branch git fetch origin pull/6180/head:model-system-rework + git checkout model-system-rework
  3. Run gradlew setup
  4. Run gradlew publishToMavenLocal
  5. Go to <forge folder>/projects/forge/build/libs/ and write down the exact version number eg. forge-1.14.4-28.1.90-model-system-rework.jar
  6. Edit your project's build.gradle, and in your repositories (NOT the repositories block from buildscript!!), add mavenLocal() as a repository, eg.
repositories {
    mavenLocal()
}
  1. Edit your minecraft dependency to match the version as seen in the build results, eg.
minecraft 'net.minecraftforge:forge:1.14.4-28.1.90-model-system-rework'
  1. Refresh/reimport your gradle project into your IDE, and wait for the setup to complete.
  2. Rerun gradlew gen<YOURIDE>Runs
  3. Implement your models under the new system and tell me of any issue you encounter or any feature you think would be important to add.
@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Sep 22, 2019

Showcase:

test fluid bucket

This is the test fluid bucket, which I gave a model json that uses the new system.

@gigaherz gigaherz requested a review from tterrag1098 Sep 22, 2019
@gigaherz gigaherz force-pushed the gigaherz:model-system-rework branch from 1ca2304 to fbd2383 Oct 7, 2019
@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Oct 7, 2019

Status so far:

  • Modified BlockModel to redirect parsing model files to the new loader ✔️
    • Expanded BlockModel to hold the (optional) custom model ✔️
      • Model jsons now support a "loader" key as an alternative to the vanilla "elements" key. ✔️
    • Expanded BlockModel to hold the (optional) forge-style transforms data ✔️
      • Model jsons now support a "transform" key as an alternative to the vanilla "display" key. ✔️
  • Implemented loader support for using existing old-style model loaders from model jsons ✔️
  • Implemented brand new OBJ model loader designed for the new system ✔️
  • Adapted existing ModelDynBucket to handle the new model system API ✔️
  • Tested that the new model loader system does not prevent vanilla models from loading correctly ✔️
  • Tested that the new model loader system does not prevent forge blockstates models from loading correctly ✔️
@gigaherz gigaherz marked this pull request as ready for review Oct 7, 2019
@williewillus

This comment has been minimized.

Copy link
Contributor

williewillus commented Oct 7, 2019

I guess removing all the *2 classes isnt possible without breaking compat, in which case you volunteer to clean it up next major version ;P

Otherwise, this looks great from a user interface point of view. Could you maybe give a brief high-level rundown of what code changes were done?

@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Oct 7, 2019

Yes the goal is to get rid of the old model loading system in 1.15, and rename those *2 classes.

EDIT: Err premature sending.

As for code changes... Note that the PR has more changes unrelated to the model system because #6154 has not been merged yet. This includes changes to BlockModelDefinition, ModelBakery, Variant, BlockStateLoader, ModelLoader, ...

I have outlined the changes to BlockModel in the comment above, which is where most of the hooks reside in.
I have expanded IForgeBakedModel to include a doesHandlePerspective getter, which needs to be overriden to true in order to avoid the extra wrapping of the model into a PerspectiveMapWrapper.
I have moved the TRSR transform deserializer logic to the new model loader class, so that it doesn't get lost when cleaning up next version.
I have provided actual PNG textures for forge:white (used to be a custom sprite), masks for the new vanilla bucket icon.

@williewillus

This comment has been minimized.

Copy link
Contributor

williewillus commented Oct 7, 2019

Are there plans to add some common model types similar to the old forge ones?

One I can think of already is just composing two other models together with individual transforms for each

@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Oct 7, 2019

I didn't have any plans for it, but that doesn't mean it can't be done. :P

@gigaherz gigaherz force-pushed the gigaherz:model-system-rework branch from 579c21c to d2e0633 Oct 7, 2019
@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Oct 7, 2019

Rebased on top of latest forge, now that I merged #6154

This screenshot shows:

  • a working bucket model
  • the same model displaying increased size from the forge-style transforms
  • blockstates-based "obsidian wall" (not a fully working block, only a test)
  • OBJ model in inventory and head.

image

@gigaherz gigaherz force-pushed the gigaherz:model-system-rework branch from d2e0633 to 2844ce4 Oct 18, 2019
@gigaherz gigaherz force-pushed the gigaherz:model-system-rework branch from 2844ce4 to 9fade4e Oct 29, 2019
@Alpvax

This comment has been minimized.

Copy link

Alpvax commented Nov 6, 2019

Is it just me, or is that bucket huge? If so, is that intentional?

@TeamSpen210

This comment has been minimized.

Copy link

TeamSpen210 commented Nov 6, 2019

That's the default size of a model, without the item transformation settings. That wouldn't be important though.

@Alpvax

This comment has been minimized.

Copy link

Alpvax commented Nov 6, 2019

My question was is there an error with the size, the answer is no. Thanks

@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Nov 14, 2019

I have updated the top description to include steps to help people test.

@KlemenDEV

This comment has been minimized.

Copy link

KlemenDEV commented Nov 14, 2019

I have tested your branch and found out the following:

  • the old way to load OBJ models still works
  • vanilla model loading is not broken
  • the new bucket JSON format works
  • new way of OBJ loading did not work for me (tested with item model)

I have tried to load OBJ model for item. The item model JSON code is:

{
  "parent": "forge:item/default",
  "loader": "forge:obj",
  "model": "test:models/item/item_direction.obj",
  "textures": {
    "particle": "test:items/particle.png"
  },
  "transform": "forge:default-item"
}

The error I see in the client run log is:

Unable to load model: 'test:obj#inventory' referenced from: test:obj#inventory: java.lang.RuntimeException: Could not read OBJ model

The OBJ and MTL files are attached in the ZIP. OBJ file is loaded properly with the old OBJ loader.

item_direction.zip

@KlemenDEV

This comment has been minimized.

Copy link

KlemenDEV commented Nov 14, 2019

Now OBJ model works just fine, and other things I have tested and listed above too.

@gigaherz gigaherz force-pushed the gigaherz:model-system-rework branch from b9ca050 to efab3b9 Nov 15, 2019
@AzureZhen

This comment has been minimized.

Copy link

AzureZhen commented Nov 15, 2019

Works for great for OBJ's that just use the MTL to get the textures and even get the multiple textures from the MTL. No code needed, just use the "loader": "forge:obj", and set it up correctly. Just have to update all of my transforms from 1.12. 596 jsons more to go!
image

@nikita488

This comment has been minimized.

Copy link
Contributor

nikita488 commented Nov 15, 2019

It is possible to add custom json attribute to OBJModel that will specify group that will be renderer and not all groups from obj file? Like for example i have Crystal obj file with 6 different groups (based on amount of crystals in block) and i want to render just one of them, not all. Without this i need to split my obj file to 6 files.

@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Nov 15, 2019

I really want to avoid coding logic for that. It makes everything a huge mess for little gain.
Your specific use case however does have some merit.

I set up the system so that part information is available internally, for loaders that produce multi-part models. I suppose I could maybe set up a "part predicate" system, that any loader can make use of. I'll look into it and see if I can come up with a part whitelist/blacklist system that isn't a mess.

@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Nov 15, 2019

@nikita488 I hope this satisfies your needs. :P

gigaherz added 24 commits Oct 7, 2019
…ement in this iteration of the model system.
…ISimpleModelGeometry, it made no sense to have it.
…perspectives) not being applied to the models.

Add loader for vanilla "elements" data, that goes through the geometry system.
Update license headers.
- Added a concept of model meshes, which fixes models that have changes of material in the middle of an object/group.
- Added a flag to enable using the ambient color as an indicator of fullbright quads.
- Improved caching key, to avoid issues when reusing models with different settings.
General fixes and improvements:
- Fix derp in the visibility data, which for no logical reason at all, I made a static field.
- Changed visibility values from an enum, to just a plain boolean.
@gigaherz gigaherz force-pushed the gigaherz:model-system-rework branch from 32e6ea7 to 744eb57 Nov 24, 2019
@desht

This comment has been minimized.

Copy link
Contributor

desht commented Nov 25, 2019

I noticed that the forge:bucket loader doesn't appear to account for fluid colours, as defined by FluidAttributes.Builder#color(). This attribute does affect the rendering of the fluid in-world, but the bucket item always renders the fluid texture exactly as it was loaded.

@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Nov 26, 2019

Oops forgot to reply earlier, I wasn't home. The bucket model has a "applyTint" value, which I mistakenly made default false. I'll fix.

@desht

This comment has been minimized.

Copy link
Contributor

desht commented Nov 29, 2019

Just a quick comment to say that tinted OBJ models appear to work very nicely:
2019-11-28_23 57 11
The red & blue sections on that Vortex Tube are OBJ-tinted and vary depending on the temperature of each end of the tube. Works great!

@KlemenDEV

This comment has been minimized.

Copy link

KlemenDEV commented Dec 7, 2019

@gigaherz Are there any updates on when this feature branch could be merged with master 1.14.4 branch so these features could be used in production mods?

@gigaherz

This comment has been minimized.

Copy link
Collaborator Author

gigaherz commented Dec 7, 2019

Considering it's 3-4 days until the release of 1.15, this will most probably be merged straight into the 1.15 branch, and I'll take care of any remaining polish/bugs during the 1.15 testing phase. Whether or not I'll try to also get it into 1.14, depends on if 1.14.4 becomes the next LTS version, or 1.12.2 remains LTS until 1.16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.