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

Keyboard #659

Open
wants to merge 17 commits into
base: dev/0.8
Choose a base branch
from
Open

Keyboard #659

wants to merge 17 commits into from

Conversation

SirEndii
Copy link
Member

@SirEndii SirEndii commented Sep 23, 2024

  • Please check if the PR fulfills these requirements
  • The commit message are well described
  • Docs have been added / updated
  • All changes have fully been tested
  • What kind of change does this PR introduce? (Bug fix, feature, ...)
    Feature

  • What is the new behavior (if this is a feature change)?
    Added the keyboard item. An item which can be linked to a computer. A user can now right click the item to unlock their mouse, they can now freely type while the typed characters are send to the linked computer. Will also be a module for the smart glasses.

  • Does this PR introduce a breaking change? (What changes might users need to make in their scripts due to this PR?)
    No

  • Other information:

@SirEndii SirEndii self-assigned this Sep 23, 2024
@SirEndii SirEndii added enhancement New feature or request 1.19x labels Sep 23, 2024
@SirEndii SirEndii added this to the 0.8r milestone Sep 23, 2024
@SirEndii SirEndii changed the title [IntelligenceModding/Advanced-Peripherals-Features#59] Keyboard item and simple screen to unlock the mouse Keyboard Sep 23, 2024
@SirEndii
Copy link
Member Author

So the keyboard works now
I can bound it to a computer, right click it in the air and start typing. Every key stroke, key presses and releases are synced to the terminal

I now want to remove JEI from the keyboard screen and place a little info text at the top

The last thing I need to do is to implement this as a smart glasses module

…dule keybind. I will see what I will do with the hotkey module or if I will create a new Keybind

I've created two new nbt tags to bind the module to the glasses to sync that to the container, I am not sure if there is maybe a better way, but it works
@SirEndii
Copy link
Member Author

I think everything is done now. Mouse and keyboard events are working, I got the little info text rendered, JEI does not render anymore and everything works fine as a glasses module

@SirEndii SirEndii marked this pull request as ready for review September 30, 2024 10:21
@SirEndii SirEndii requested a review from zyxkad September 30, 2024 10:22
Copy link
Collaborator

@zyxkad zyxkad left a comment

Choose a reason for hiding this comment

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

Overall looks good, just have some detail need to deal with

@SirEndii
Copy link
Member Author

Sorry to annoy you @SquidDev, just asking you because of #650 (comment)

I used a piece of the TerminalWidget implementation for our keyboard screen. Just asking if this is okay
See
https://github.com/IntelligenceModding/AdvancedPeripherals/pull/659/files#diff-6374267185184dd9bdd6868cb47752c688a105088d0063d0c5bf7992b4e10ec9R19

And maybe you can answer the two comments related to that from zyxkad

@SquidDev
Copy link
Contributor

It might be easier to copy what NoTermComputerScreen does, and just create a Terminal but never render it. Plethora's keyboard UI actually just subclasses NoTermComputerScreen directly.

@SirEndii
Copy link
Member Author

It might be easier to copy what NoTermComputerScreen does, and just create a Terminal but never render it. Plethora's keyboard UI actually just subclasses NoTermComputerScreen directly.

Thank you, I didn't see that
That would be simpler

Copy link
Contributor

github-actions bot commented Jan 16, 2025

Build Preview

badge

You can find files attached to the below linked Workflow Run URL (Logs).

Name Link
Commit 1c9a485
Logs https://github.com/IntelligenceModding/AdvancedPeripherals/actions/runs/12801308318
Jar Files AdvancedPeripherals PR 659
Expires At 2025-04-16T03:07:38Z

@SirEndii SirEndii requested a review from zyxkad January 16, 2025 03:07
build.gradle Outdated
@@ -324,11 +324,12 @@ dependencies {
compileOnly fg.deobf("com.ldtteam:domum_ornamentum:${domumornamentum_version}:universal")
compileOnly fg.deobf("com.ldtteam:blockui:${blockui_version}")
// IMPORTANT. This should be removed/commented when running `runData`
runtimeOnly fg.deobf("com.ldtteam:minecolonies:${minecolonies_version}")
/*runtimeOnly fg.deobf("com.ldtteam:minecolonies:${minecolonies_version}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I pretend I didn't see those

(although I think you should merge from dev/0.8)

* <p>
* We just create a terminal which is used to forward all the key presses and mouse clicks but we don't render it.
*/
public class KeyboardScreen extends BaseScreen<KeyboardContainer> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you do not extend from AbstractContainerScreen, but inherit Screen or GuiComponent, then jade should not render on it

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public class KeyboardContainer extends BaseContainer implements ComputerMenu {
Copy link
Collaborator

Choose a reason for hiding this comment

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

And so we don't need KeyboardContainer anymore

if (data.contains("ownerId") && data.contains("owner")) {
tooltip.add(EnumColor.buildTextComponent(Component.translatable("item.advancedperipherals.tooltip.memory_card.bound", data.getString("owner"))));
if (data.contains("ownerId")) {
tooltip.add(EnumColor.buildTextComponent(Component.translatable("item.advancedperipherals.tooltip.binding.boundto", data.getString("ownerId"))));
Copy link
Collaborator

Choose a reason for hiding this comment

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

alr we are not showing ownerId here, we still want show the owner's name, so we need an extra step to turn the id into GameProfile, then get the player's cached name

Copy link
Member Author

Choose a reason for hiding this comment

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

ah yeah
I can use the new cache thing there I made

SirEndii and others added 3 commits January 17, 2025 22:48
Co-authored-by: Kevin Z <[email protected]>
Signed-off-by: Srendi <[email protected]>
# Conflicts:
#	build.gradle
#	src/generated/resources/.cache/67cce32b1c3cbbcb1f646605f4914e3f196986c2
#	src/generated/resources/.cache/c622617f6fabf890a00b9275cd5f643584a8a2c8
#	src/generated/resources/assets/advancedperipherals/lang/en_us.json
#	src/main/java/de/srendi/advancedperipherals/common/data/EnUsLanguageProvider.java
#	src/main/java/de/srendi/advancedperipherals/common/setup/APItems.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.19x enhancement New feature or request
Projects
Status: Needs testing/review
Development

Successfully merging this pull request may close these issues.

3 participants