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

Forge 1.16.4 - Item mod element #554

Closed
wants to merge 19 commits into from
Closed

Forge 1.16.4 - Item mod element #554

wants to merge 19 commits into from

Conversation

Goldorion
Copy link
Collaborator

@Goldorion Goldorion commented Nov 22, 2020

This PR adds the item and food mod elements to the 1.16.4 Forge generator.

Issue #4

Goldorion and others added 13 commits October 23, 2020 08:05
Fixed food not healing hunger if they had a result item and made the …
* Improved help tip to resolve confusion as appeared in https://mcreator.net/forum/67868/act-ladders-not-working

Signed-off-by: KlemenDEV <klemen.pylo@gmail.com>

* Updated some dependencies to fix some bugs in the used libs and improve stability

Signed-off-by: KlemenDEV <klemen.pylo@gmail.com>

* New translations texts.properties (French) (#510)

* Fix stamp tool offset.

* Externalized Blockly panels components and reorganized some strings (#495)

* Externalized CompileNotesPanel

* Externalized default trigger

* Removed duplicate intro comment from ProcedureTemplateDropdown

* Externalized AITasksEditorToolbar

* Externalized ProcedureEditorToolbar

* Added new keys and reorganized some existing strings

* Added new strings

Co-authored-by: KlemenDEV <klemen.pylo@gmail.com>

* Clarified xp result (MCreator/Generator-Forge-1.12.2#20)

Signed-off-by: KlemenDEV <klemen.pylo@gmail.com>

* New Crowdin updates (#512)

* New translations texts.properties (French)

* New translations texts.properties (Portuguese, Brazilian)

* New translations texts.properties (Chinese Simplified)

* New translations texts.properties (Chinese Traditional)

* New translations texts.properties (German)

* New translations texts.properties (Russian)

* New translations texts.properties (Vietnamese)

* New translations texts.properties (Polish)

* Fixed #507 ("Affected by fortune" checkbox under loot tables is not working) + improved loot table tests

Signed-off-by: KlemenDEV <klemen.pylo@gmail.com>

* New texture templates (#514)

* Code reformat, fixed one wrong externalization use case

Signed-off-by: KlemenDEV <klemen.pylo@gmail.com>

* New Crowdin updates (#517)

* New translations texts.properties (French)

* New translations texts.properties (Portuguese, Brazilian)

* New translations texts.properties (Chinese Simplified)

* New translations texts.properties (Chinese Traditional)

* New translations texts.properties (Russian)

* New translations texts.properties (Vietnamese)

* New translations texts.properties (Polish)

* New Crowdin updates (#524)

* New translations texts.properties (Portuguese, Brazilian)

* New translations texts.properties (German)

* New translations texts.properties (Vietnamese) (#527)

* New translations texts.properties (French) (#533)

* New translations texts.properties (Polish) (#536)

* Fixed #535

Signed-off-by: KlemenDEV <klemen.pylo@gmail.com>

* Removed help button that appeared in procedure editor after Blockly update (Fixes #538)

Signed-off-by: KlemenDEV <klemen.pylo@gmail.com>

* Fixed datalist mistake

Signed-off-by: KlemenDEV <klemen.pylo@gmail.com>

* New translations texts.properties (Vietnamese) (#541)

Co-authored-by: KlemenDEV <klemen.pylo@gmail.com>
Co-authored-by: Matej <matej2000@gmail.com>
Co-authored-by: Defeatomizer <drag7398@gmail.com>
# Conflicts:
#	plugins/generator-addon-1.16.x/addon-1.16.x/templates/loottable.json.ftl
#	plugins/generator-addon-1.16.x/addon-1.16.x/templates/resourcepack/sound_definitions.json.ftl
# Conflicts:
#	plugins/generator-1.14.4/forge-1.14.4/templates/json/loottable.json.ftl
#	plugins/generator-1.14.4/forge-1.14.4/templates/mob.java.ftl
@Goldorion Goldorion added this to the 1.16.x support milestone Nov 22, 2020
Copy link
Contributor

@KlemenDEV KlemenDEV left a comment

Choose a reason for hiding this comment

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

This PR seems to have some files not related to the PR changed, please remove these / revert changes.

Did you test if OBJ and JSON models work properly? Also, make sure to test as many configuration combinations as possible, also test all triggers.

Split food into new PR too, I asked for one mod element per PR so we keep everything organized.

Also please wait with new features for now, first we need to update everything ;)

@Goldorion
Copy link
Collaborator Author

Goldorion commented Nov 22, 2020

This PR seems to have some files not related to the PR changed, please remove these / revert changes.

You merged #551 just before I created the PR, so I had still the old changes. It should be ok now.

Did you test if OBJ and JSON models work properly? Also, make sure to test as many configuration combinations as possible, also test all triggers.

I have no OBJ models and I don't know how to make one (a software), but I can test for the JSON models.

Split food into new PR too, I asked for one mod element per PR so we keep everything organized.

I put them in the same PR because both use the same class. They are only a few extra methods inside the Properties of the food elements.

Also please wait with new features, for now, first we need to update everything ;)

Oh ok, sorry :)

@KlemenDEV
Copy link
Contributor

I have no OBJ models and I don't know how to make one (a software), but I can test for the JSON models.

Indeed test JSON models to make sure nothing changed, for OBJ, use this one:
obj.zip

I put them in the same PR because both use the same class. They are only a few extra methods inside the Properties of the food elements.

It is to keep things organized, so we can quickly manage things, revert single feature, and to track feature changes per commits. It just is a good practice.

Oh ok, sorry :)

I updated #4 with guidelines to streamline this :)

@Goldorion
Copy link
Collaborator Author

It is to keep things organized, so we can quickly manage things, revert single feature, and to track feature changes per commits. It just is a good practice.

Should I create a new PR for the food or is it ok for this PR?

@KlemenDEV
Copy link
Contributor

KlemenDEV commented Nov 22, 2020

It is to keep things organized, so we can quickly manage things, revert single feature, and to track feature changes per commits. It just is a good practice.

Should I create a new PR for the food or is it ok for this PR?

I would prefer two new PRs for food and the new item feature, and then keep this one for the original item porting.

@KlemenDEV KlemenDEV marked this pull request as draft November 22, 2020 18:03
@KlemenDEV
Copy link
Contributor

KlemenDEV commented Nov 22, 2020

I converted this to draft. Needed work:

  • split food and new item feature to two more PRs
  • fix OBJ model issues
  • some unit tests fail due to missing mappings (this will work once Matej finished them and you merge them in your working branch), but there are some issues due to not features being tested. Please, really do in-depth testing, otherwise, I have to test everything again.

@Goldorion Goldorion changed the title Forge 1.16.4 - Items + Is immune to fire Forge 1.16.4 - Item mod element Nov 22, 2020
@Goldorion Goldorion removed the request for review from KlemenDEV November 22, 2020 20:49
@KlemenDEV
Copy link
Contributor

Just to note for other contributors, OBJ works fine, the supplied demo OBJ file has a typo in material file reference.

@KlemenDEV
Copy link
Contributor

Block/item mappings are done so I suggest merging master into this PR to re-run tests so we can see if they pass.

@Goldorion Goldorion marked this pull request as ready for review November 25, 2020 15:55
.idea/misc.xml Outdated Show resolved Hide resolved
This reverts commit 7c32185.
@KlemenDEV KlemenDEV self-requested a review November 25, 2020 18:16
@Max094Reikeb
Copy link
Collaborator

Travis build failed btw

@KlemenDEV KlemenDEV marked this pull request as draft November 27, 2020 09:42
@KlemenDEV
Copy link
Contributor

Is this PR planned to being worked on? Or some other contributor ready to pick it up?

@KlemenDEV
Copy link
Contributor

This PR got orphaned because original fork was deleted.

@Goldorion, please open new PR for this from an active fork, as I can't push to this PR anymore, nor can anyone else.

@KlemenDEV KlemenDEV closed this Dec 2, 2020
@MrScautHD
Copy link

i think i can help. the old 1.15.2 Code is working very fine with the 1.16.4 version
ads

@KlemenDEV
Copy link
Contributor

i think i can help. the old 1.15.2 Code is working very fine with the 1.16.4 version
ads

There is another contributor working on it. You need to port attributes and probably inventory for 1.16.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants