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

Feature/travel staff #18

Closed
justliliandev wants to merge 5 commits from feature/travel_staff into dev/1.18.x
pull from: feature/travel_staff
merge into: Team-EnderIO:dev/1.18.x
Conversation 13 Commits 5 Files changed 28 +993 -21
justliliandev commented 2022-01-10 23:37:21 +00:00 (Migrated from github.com)
Copy link

Description

Added Staff of Travelling
Added TeleportationTargetSystem using WorldSavedData (can be moved to capability, if wanted, but WorldSavedData is imo the better approach for this)
Marked as a Draft, because I wanted opinions on:
API-Annotation
TravelRegistry allowing Mods to add their own TravelTargets without a dependency on machines
Usage of IMC vs API anotated methods/class vs API package
Every Travel Target defines how it renders

Todo

  • GUI of Travel Anchor
  • Staff of Travelling Targetting Travel Targets
  • Maybe Rendering, but I would like to put that on Crazy :)

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 Added Staff of Travelling Added TeleportationTargetSystem using WorldSavedData (can be moved to capability, if wanted, but WorldSavedData is imo the better approach for this) Marked as a Draft, because I wanted opinions on: API-Annotation TravelRegistry allowing Mods to add their own TravelTargets without a dependency on machines Usage of IMC vs API anotated methods/class vs API package Every Travel Target defines how it renders # Todo - [ ] GUI of Travel Anchor - [ ] Staff of Travelling Targetting Travel Targets - [ ] Maybe Rendering, but I would like to put that on Crazy :) # 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 -->
CrazyPants (Migrated from github.com) reviewed 2022-01-15 13:26:01 +00:00
enderio-base/src/main/java/com/enderio/base/client/ClientEventHandler.java
CrazyPants (Migrated from github.com) commented 2022-01-15 08:30:12 +00:00
Copy link

I would rename addParticles and getColor to something like 'addTravelParticles' etc
Depending on much ends up being in this class I would even consider moving the travel stuff into a sperate class compleletly. The way it is it could be pretty hard to find where this functionality is implemented

I would rename addParticles and getColor to something like 'addTravelParticles' etc Depending on much ends up being in this class I would even consider moving the travel stuff into a sperate class compleletly. The way it is it could be pretty hard to find where this functionality is implemented
enderio-base/src/main/java/com/enderio/base/client/renderer/teleportation/RenderTeleportTargets.java
@ -0,0 +25,4 @@
Vec3 projectedView = mainCamera.getPosition();
poseStack.translate(-projectedView.x, -projectedView.y, -projectedView.z);
//TODO: Scale, View Bobbing, (FOV/Creative Flight offset stuff)
CrazyPants (Migrated from github.com) commented 2022-01-15 08:31:43 +00:00
Copy link

Sure, this stuff was pretty complex to get looking the way I wanted from memory. Was the scaling based on screen pos etc not done in the ported mod?

Sure, this stuff was pretty complex to get looking the way I wanted from memory. Was the scaling based on screen pos etc not done in the ported mod?
enderio-base/src/main/java/com/enderio/base/common/handler/travel/TeleportHandler.java
@ -0,0 +1,156 @@
package com.enderio.base.common.handler.travel;
CrazyPants (Migrated from github.com) commented 2022-01-15 08:38:39 +00:00
Copy link

Using this method is the sound played to other people close to the player? You are also playing the sound even if the teleport doesn't happen.

Using this method is the sound played to other people close to the player? You are also playing the sound even if the teleport doesn't happen.
CrazyPants (Migrated from github.com) commented 2022-01-15 08:43:28 +00:00
Copy link

You add the player eye height above but then just use target.below() for the ground check. This will work in most cases but it is probably more correct to subtract eye height again

You add the player eye height above but then just use target.below() for the ground check. This will work in most cases but it is probably more correct to subtract eye height again
CrazyPants (Migrated from github.com) commented 2022-01-15 08:48:22 +00:00
Copy link

I would rename this to isTelportPositionClear or something. The name sounds like it is testing if the target pos is actual 'ground', not just checking for empty space. On initial reading I though you couldn't teleport to a position in mid air.

I would rename this to isTelportPositionClear or something. The name sounds like it is testing if the target pos is actual 'ground', not just checking for empty space. On initial reading I though you couldn't teleport to a position in mid air.
enderio-base/src/main/java/com/enderio/base/common/handler/travel/TeleportHandler.java
@ -0,0 +38,4 @@
return true;
if (stack.getItem() instanceof IDarkSteelItem darkSteelItem) {
//TODO: Check for upgrade;
}
CrazyPants (Migrated from github.com) commented 2022-01-15 08:37:45 +00:00
Copy link

Are we going to allow this to be extensible by other mods? I am not fussed if we don't bother with this for now.
We could add an ITraveltem interface (the pair of ITravelTarget). This would need a method like 'isAvailable' to handle whether an upgrade is available or not.
This method also doesn't consider if you have enough energy in the item to teleport but I am guessing that is the correct

Are we going to allow this to be extensible by other mods? I am not fussed if we don't bother with this for now. We could add an ITraveltem interface (the pair of ITravelTarget). This would need a method like 'isAvailable' to handle whether an upgrade is available or not. This method also doesn't consider if you have enough energy in the item to teleport but I am guessing that is the correct
enderio-base/src/main/java/com/enderio/base/common/item/tool/TravelStaffItem.java
@ -0,0 +37,4 @@
@Override
public InteractionResultHolder<ItemStack> use(Level level, Player player, InteractionHand usedHand) {
ItemStack stack = player.getItemInHand(usedHand);
if (getActivationStatus(stack).isAir()) {
CrazyPants (Migrated from github.com) commented 2022-01-15 08:55:45 +00:00
Copy link

Both 'use' and 'useOn' should be able to do both teleport types. Most of the time you wish to travel to a block you will right click the air which will come through this method.

Both 'use' and 'useOn' should be able to do both teleport types. Most of the time you wish to travel to a block you will right click the air which will come through this method.
enderio-base/src/main/java/com/enderio/base/common/item/tool/TravelStaffItem.java
@ -0,0 +47,4 @@
}
@Override
public InteractionResult useOn(UseOnContext context) {
CrazyPants (Migrated from github.com) commented 2022-01-15 09:03:19 +00:00
Copy link

So you need to make sure the block you are right clicking on hasn't 'consumed' the right click before doing the teleport? Can't remember how this works.

So you need to make sure the block you are right clicking on hasn't 'consumed' the right click before doing the teleport? Can't remember how this works.
enderio-machines/src/main/java/com/enderio/machines/common/blockentity/TravelAnchorBlockEntity.java
@ -0,0 +23,4 @@
public TravelAnchorBlockEntity(BlockEntityType<?> pType, BlockPos pWorldPosition, BlockState pBlockState) {
super(pType, pWorldPosition, pBlockState);
add2WayDataSlot(new StringDataSlot(this::getName, this::setName, SyncMode.GUI));
CrazyPants (Migrated from github.com) commented 2022-01-15 09:06:30 +00:00
Copy link

We probably shouldn't be using a number in the middle of a method name like this

We probably shouldn't be using a number in the middle of a method name like this
justliliandev (Migrated from github.com) reviewed 2022-01-15 13:32:13 +00:00
enderio-base/src/main/java/com/enderio/base/common/item/tool/TravelStaffItem.java
@ -0,0 +47,4 @@
}
@Override
public InteractionResult useOn(UseOnContext context) {
justliliandev (Migrated from github.com) commented 2022-01-15 13:32:13 +00:00
Copy link

the method is only called if the block hasn't consumed the right click

the method is only called if the block hasn't consumed the right click
justliliandev (Migrated from github.com) reviewed 2022-01-15 13:32:30 +00:00
enderio-machines/src/main/java/com/enderio/machines/common/blockentity/TravelAnchorBlockEntity.java
@ -0,0 +23,4 @@
public TravelAnchorBlockEntity(BlockEntityType<?> pType, BlockPos pWorldPosition, BlockState pBlockState) {
super(pType, pWorldPosition, pBlockState);
add2WayDataSlot(new StringDataSlot(this::getName, this::setName, SyncMode.GUI));
justliliandev (Migrated from github.com) commented 2022-01-15 13:32:30 +00:00
Copy link

do you have a better name for it?

do you have a better name for it?
justliliandev (Migrated from github.com) reviewed 2022-01-15 13:42:48 +00:00
enderio-base/src/main/java/com/enderio/base/common/handler/travel/TeleportHandler.java
@ -0,0 +38,4 @@
return true;
if (stack.getItem() instanceof IDarkSteelItem darkSteelItem) {
//TODO: Check for upgrade;
}
justliliandev (Migrated from github.com) commented 2022-01-15 13:42:48 +00:00
Copy link

The ITravelTarget interface can be used as an optional dependency, because it doesn't need to be implemented on minecraft exposed registry objects like item and blocks. Forcing an interface on Items or Blocks (what ctm is doing rn), would make our mod to be a required dependency. This method is only called for particle rendering and other clients can add their own particles or use our addParticleMethod, if we make it public

The ITravelTarget interface can be used as an optional dependency, because it doesn't need to be implemented on minecraft exposed registry objects like item and blocks. Forcing an interface on Items or Blocks (what ctm is doing rn), would make our mod to be a required dependency. This method is only called for particle rendering and other clients can add their own particles or use our addParticleMethod, if we make it public
justliliandev (Migrated from github.com) reviewed 2022-01-15 13:44:47 +00:00
enderio-base/src/main/java/com/enderio/base/client/renderer/teleportation/RenderTeleportTargets.java
@ -0,0 +25,4 @@
Vec3 projectedView = mainCamera.getPosition();
poseStack.translate(-projectedView.x, -projectedView.y, -projectedView.z);
//TODO: Scale, View Bobbing, (FOV/Creative Flight offset stuff)
justliliandev (Migrated from github.com) commented 2022-01-15 13:44:47 +00:00
Copy link

it wasn't as they use a BlockEntityRenderer for some things, which wouldn't work for us, because we have the big teleporter with infinite range. Also I wanted to get the logic done first, so the Draft would be cleaner

it wasn't as they use a BlockEntityRenderer for some things, which wouldn't work for us, because we have the big teleporter with infinite range. Also I wanted to get the logic done first, so the Draft would be cleaner
justliliandev (Migrated from github.com) reviewed 2022-01-15 13:47:30 +00:00
enderio-base/src/main/java/com/enderio/base/client/ClientEventHandler.java
justliliandev (Migrated from github.com) commented 2022-01-15 13:47:30 +00:00
Copy link

I've put it there, because I would need another TeleportHandler class just for the client stuff, because of classloading issues. Can extract that to another class called TeleportClientHandler but the code is really seperate anyway, so I don't see a big benefit for extracting it out

I've put it there, because I would need another TeleportHandler class just for the client stuff, because of classloading issues. Can extract that to another class called TeleportClientHandler but the code is really seperate anyway, so I don't see a big benefit for extracting it out
tyler489 (Migrated from github.com) reviewed 2022-01-15 14:50:03 +00:00
enderio-machines/src/main/java/com/enderio/machines/common/blockentity/TravelAnchorBlockEntity.java
@ -0,0 +23,4 @@
public TravelAnchorBlockEntity(BlockEntityType<?> pType, BlockPos pWorldPosition, BlockState pBlockState) {
super(pType, pWorldPosition, pBlockState);
add2WayDataSlot(new StringDataSlot(this::getName, this::setName, SyncMode.GUI));
tyler489 (Migrated from github.com) commented 2022-01-15 14:50:03 +00:00
Copy link

addTwoWayDataSlot

addTwoWayDataSlot
CrazyPants (Migrated from github.com) reviewed 2022-01-15 15:25:55 +00:00
enderio-machines/src/main/java/com/enderio/machines/common/blockentity/TravelAnchorBlockEntity.java
@ -0,0 +23,4 @@
public TravelAnchorBlockEntity(BlockEntityType<?> pType, BlockPos pWorldPosition, BlockState pBlockState) {
super(pType, pWorldPosition, pBlockState);
add2WayDataSlot(new StringDataSlot(this::getName, this::setName, SyncMode.GUI));
CrazyPants (Migrated from github.com) commented 2022-01-15 15:25:55 +00:00
Copy link

Yep

Yep
CrazyPants (Migrated from github.com) reviewed 2022-01-15 15:27:44 +00:00
enderio-base/src/main/java/com/enderio/base/common/handler/travel/TeleportHandler.java
@ -0,0 +38,4 @@
return true;
if (stack.getItem() instanceof IDarkSteelItem darkSteelItem) {
//TODO: Check for upgrade;
}
CrazyPants (Migrated from github.com) commented 2022-01-15 15:27:44 +00:00
Copy link

Sounds good

Sounds good
CrazyPants (Migrated from github.com) reviewed 2022-01-15 16:46:37 +00:00
enderio-base/src/main/java/com/enderio/base/client/ClientEventHandler.java
CrazyPants (Migrated from github.com) commented 2022-01-15 16:46:36 +00:00
Copy link

The only value I see is that all the teleport functionality would then be in a single package. Someone looking at the teleport code for the first time might have a hard time finding this functionality.
Just a suggestion though, I am happy for it to stay where it is.

The only value I see is that all the teleport functionality would then be in a single package. Someone looking at the teleport code for the first time might have a hard time finding this functionality. Just a suggestion though, I am happy for it to stay where it is.
justliliandev (Migrated from github.com) reviewed 2022-01-30 19:53:14 +00:00
enderio-base/src/main/java/com/enderio/base/common/handler/travel/TeleportHandler.java
@ -0,0 +1,156 @@
package com.enderio.base.common.handler.travel;
justliliandev (Migrated from github.com) commented 2022-01-30 19:53:14 +00:00
Copy link

it isn't played to other players, should other players nearby hear it?

it isn't played to other players, should other players nearby hear it?
justliliandev (Migrated from github.com) reviewed 2022-01-30 19:53:21 +00:00
enderio-base/src/main/java/com/enderio/base/client/ClientEventHandler.java
justliliandev (Migrated from github.com) commented 2022-01-30 19:53:21 +00:00
Copy link

moved it

moved it
Rover656 commented 2023-01-17 14:22:22 +00:00 (Migrated from github.com)
Copy link

Closing in favor of #103

Closing in favor of #103

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
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#18
No description provided.
Delete branch "feature/travel_staff"

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?