Team-EnderIO/EnderIO
1
0
Fork 0
Code Issues 114 Pull requests 13 Projects Releases 79 Packages Wiki Activity Actions

Soul Data (for soul bound machines and items) #119

Merged
ferriarnus merged 51 commits from soul-config into dev/1.20.1 2023-07-02 13:14:53 +00:00
Conversation 12 Commits 51 Files changed 98 +998 -45
ferriarnus commented 2023-03-31 14:45:23 +00:00 (Migrated from github.com)
Copy link

Description

Adds a json codec based system, similar to recipes. It collects them as a map of the entitytype and a record of the wanted data. It contains an example using the powered spawner.

Todo

  • Check implementation quality

Checklist:

  • My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
# Description Adds a json codec based system, similar to recipes. It collects them as a map of the entitytype and a record of the wanted data. It contains an example using the powered spawner. # Todo <!-- Remove this section if you're submitting an already-complete PR --> - [ ] Check implementation quality # Checklist: - [ ] My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you) - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation <!-- Thanks to: https://embeddedartistry.com/blog/2017/08/04/a-github-pull-request-template-for-your-projects/ for the building blocks of this template -->
justliliandev (Migrated from github.com) reviewed 2023-03-31 14:45:23 +00:00
Rover656 (Migrated from github.com) reviewed 2023-05-06 02:39:56 +00:00
Rover656 (Migrated from github.com) left a comment
Copy link

Looks like a solid proposal thusfar, just noting my comments for you. Nice work!

Looks like a solid proposal thusfar, just noting my comments for you. Nice work!
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
@ -39,2 +49,4 @@
@Override
public void tick() {
if (entityType == null) {
complete = true;
Rover656 (Migrated from github.com) commented 2023-05-06 02:36:26 +00:00
Copy link

Might be a good idea to just store the soul datum rather than storing each field separately. Might be a neat way of doing it :)

Might be a good idea to just store the soul datum rather than storing each field separately. Might be a neat way of doing it :)
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
Rover656 (Migrated from github.com) commented 2023-05-06 02:37:03 +00:00
Copy link

Ensure that if this is the case, the machine doesn't softlock due to the job not concluding.

Ensure that if this is the case, the machine doesn't softlock due to the job not concluding.
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
@ -141,4 +184,4 @@
blockEntity.setReason(PoweredSpawnerBlockEntity.SpawnerBlockedReason.UNKOWN_MOB);
break;
}
Rover656 (Migrated from github.com) commented 2023-05-06 02:37:38 +00:00
Copy link

I would argue that only living entities should be targetable. If there is any kind of exception discovered later, we can go for adding in additional cases.

I would argue that only living entities should be targetable. If there is any kind of exception discovered later, we can go for adding in additional cases.
src/main/java/com/enderio/base/common/util/EntityCaptureUtils.java
Rover656 (Migrated from github.com) commented 2023-05-06 02:39:34 +00:00
Copy link

What would the case be for removing this? If I'm thinking about this correctly, this check will have to remain as the soul vial will not be using soul data, right?

What would the case be for removing this? If I'm thinking about this correctly, this check will have to remain as the soul vial will not be using soul data, right?
ferriarnus (Migrated from github.com) reviewed 2023-05-07 14:11:17 +00:00
src/main/java/com/enderio/base/common/util/EntityCaptureUtils.java
ferriarnus (Migrated from github.com) commented 2023-05-07 14:11:17 +00:00
Copy link

No, but I did add a blacklist tag for the soul vials as well, since I also added one for the spawner. That way other mobs can be blacklisted using the tag so this check isn't needed. There also is a bosses tag now to replace this method I think?

No, but I did add a blacklist tag for the soul vials as well, since I also added one for the spawner. That way other mobs can be blacklisted using the tag so this check isn't needed. There also is a bosses tag now to replace this method I think?
ferriarnus (Migrated from github.com) reviewed 2023-05-07 14:11:38 +00:00
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
@ -141,4 +184,4 @@
blockEntity.setReason(PoweredSpawnerBlockEntity.SpawnerBlockedReason.UNKOWN_MOB);
break;
}
ferriarnus (Migrated from github.com) commented 2023-05-07 14:11:38 +00:00
Copy link

Yeah I agree

Yeah I agree
ferriarnus (Migrated from github.com) reviewed 2023-05-07 14:12:12 +00:00
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
ferriarnus (Migrated from github.com) commented 2023-05-07 14:12:12 +00:00
Copy link

Oh good catch!

Oh good catch!
Rover656 (Migrated from github.com) reviewed 2023-05-09 12:32:30 +00:00
src/main/java/com/enderio/base/common/util/EntityCaptureUtils.java
Rover656 (Migrated from github.com) commented 2023-05-09 12:32:30 +00:00
Copy link

Wait; so there's a tag to define what mobs are bosses/blacklisted and we check that now? If so then thats perfectly fine :)

Wait; so there's a tag to define what mobs are bosses/blacklisted and we check that now? If so then thats perfectly fine :)
ferriarnus (Migrated from github.com) reviewed 2023-05-09 12:43:07 +00:00
src/main/java/com/enderio/base/common/util/EntityCaptureUtils.java
ferriarnus (Migrated from github.com) commented 2023-05-09 12:43:06 +00:00
Copy link

Yes forge added that tag some time back. I made a blacklist tag that has the forge bosses tag inside of it. So for the soul vials and spawners, both have a single tag (which has bosses as well).

Yes forge added that tag some time back. I made a blacklist tag that has the forge bosses tag inside of it. So for the soul vials and spawners, both have a single tag (which has bosses as well).
ferriarnus (Migrated from github.com) reviewed 2023-06-29 15:36:32 +00:00
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
@ -39,2 +49,4 @@
@Override
public void tick() {
if (entityType == null) {
complete = true;
ferriarnus (Migrated from github.com) commented 2023-06-29 15:31:36 +00:00
Copy link

I'm not a 100% sure on this, I found this cleaner but that's just preference

I'm not a 100% sure on this, I found this cleaner but that's just preference
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
ferriarnus (Migrated from github.com) commented 2023-06-29 15:31:32 +00:00
Copy link

would simply setting completed as true be enough here? That should end the task.

would simply setting completed as true be enough here? That should end the task.
Rover656 (Migrated from github.com) reviewed 2023-06-29 16:36:35 +00:00
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
Rover656 (Migrated from github.com) commented 2023-06-29 16:36:35 +00:00
Copy link

Yeah set completed to true to end the task

Yeah set completed to true to end the task
Rover656 (Migrated from github.com) requested changes 2023-06-30 11:50:26 +00:00
Rover656 (Migrated from github.com) left a comment
Copy link

Just a couple of pieces; nice work!

Just a couple of pieces; nice work!
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
@ -137,16 +180,18 @@ public class SpawnTask extends PoweredTask{
}
}
Rover656 (Migrated from github.com) commented 2023-06-30 11:48:39 +00:00
Copy link

Why is this commented? what is the purpose of the commented logic?

Why is this commented? what is the purpose of the commented logic?
src/main/java/com/enderio/base/common/tag/EIOTags.java
Rover656 (Migrated from github.com) commented 2023-06-29 16:45:26 +00:00
Copy link

Could you change this to EntityTypes to match EntityType :)

Could you change this to EntityTypes to match EntityType :)
ferriarnus (Migrated from github.com) reviewed 2023-06-30 11:51:50 +00:00
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
@ -137,16 +180,18 @@ public class SpawnTask extends PoweredTask{
}
}
ferriarnus (Migrated from github.com) commented 2023-06-30 11:51:49 +00:00
Copy link

Oh good catch! That shouldn't be updated, but forge made changes and I didn't get it to compile without removing it.

Oh good catch! That shouldn't be updated, but forge made changes and I didn't get it to compile without removing it.
Rover656 (Migrated from github.com) approved these changes 2023-07-02 13:09:34 +00:00
Rover656 (Migrated from github.com) left a comment
Copy link

I'm happy with how this looks, thanks!!

I'm happy with how this looks, thanks!!
Sign in to join this conversation.
Reviewers
No reviewers
justliliandev
Rover656
Labels
Clear labels   
Area-Assets

   
Area-Backend

Backend work not usually visible to players.

  
Area-Conduits

Everything involving conduits

  
Area-Datapacks

   
Area-Lang

   
Area-Mod Compat

   
Area-Parity

1.12.2 missing features

  
Area-Rendering

Rendering code, might be shaders specific or general.

  
Good first issue

   
MC-1.19.2

   
MC-1.20.1

   
MC-1.20.4

   
MC-1.20.6

   
MC-1.21

   
MC-1.21.1

   
Modtoberfest

   
P-0-High

High priority, crash/breaking.

  
P-1-Medium

Medium priority. Potentially deleting items or breaking functionality.

  
P-2-Low

Low priority. Not hurting anyone, just annoying.

  
Status-Awaiting Response

Waiting for OP to respond

  
Status-Behind-Flag

This change is available but is placed behind a feature flag.

  
Status-Blocked

Blocked by a specific issue or protocol.

  
Status-Cannot Reproduce

   
Status-Duplicate

This issue or pull request already exists

  
Status-Help Wanted

We need some help to try and track this thing down..

  
Status-Incomplete Report

   
Status-Invalid

This doesn't seem right

  
Status-Needs LTS Backport

Fixed in latest but requires LTS backport.

  
Status-Needs Updating

Needs updating to a newer Minecraft version.

  
Status-Stale

This may not be updated by the OP any further.

  
Status-To Implement

   
Status-Triage

A Triage Member should take a look at this

  
Status-Wontfix

This will not be worked on

  
Status-Wontmerge

This PR won't be merged, but is left now for reference of ongoing or future works.

  
Type-Backport

   
Type-Bug

There is a problem.

  
Type-Documentation

Improvements or additions to documentation

  
Type-Enhancement

New feature or enhancement to existing feature.

  
Type-Question

Further information is requested

  
Type-RFC

Request for comments.

  
Type-Suggestion
No labels
Area-Assets
Area-Backend
Area-Conduits
Area-Datapacks
Area-Lang
Area-Mod Compat
Area-Parity
Area-Rendering
Good first issue
MC-1.19.2
MC-1.20.1
MC-1.20.4
MC-1.20.6
MC-1.21
MC-1.21.1
Modtoberfest
P-0-High
P-1-Medium
P-2-Low
Status-Awaiting Response
Status-Behind-Flag
Status-Blocked
Status-Cannot Reproduce
Status-Duplicate
Status-Help Wanted
Status-Incomplete Report
Status-Invalid
Status-Needs LTS Backport
Status-Needs Updating
Status-Stale
Status-To Implement
Status-Triage
Status-Wontfix
Status-Wontmerge
Type-Backport
Type-Bug
Type-Documentation
Type-Enhancement
Type-Question
Type-RFC
Type-Suggestion
Milestone
Clear milestone
No items
No milestone
First Alpha Release (v6.0)
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Team-EnderIO/EnderIO#119
No description provided.
Delete branch "soul-config"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?