Conversation
…, added mic indicator, cleaned up logs, reintroduced crash fix.
|
Ooh sick, thanks for the PR! I'll debug my local environment to see why in the world it's failing, and I'll get back to you ASAP. |
|
A lot of code has been changed I see. Before going into implementation specifics, there's a couple pieces of feedback i wanna give:
I'll review each file and feature later. Thanks for the contribution, and I hope my feedback will help you. If I were to merge this PR, I'd like to do it step-by-step. First the port, then once I've tested that it fully works, the QOL features. |
5Avian
left a comment
There was a problem hiding this comment.
I've reviewed some of the files changes, but it's a lot for me to do everything at once right now. The most important points I wrote down.
If you wanna merge your code into the project, please do it in 2 steps:
- PR for port only
- PR (or issue, so we can discuss implementation) for QoL features
Also, feel free to release your own fork of this mod too. It has the MIT license, so you can basically do anything with it. Of course I'd absolutely prefer to merge port, since this is an existing project, and I'm down to discuss implementation of your QOL features. And don't let my feedback get ya down, the PR is absolutely appreciated!
src/main/java/fiveavian/proxvc/mixin/client/NetClientHandlerMixin.java
Outdated
Show resolved
Hide resolved
src/main/java/fiveavian/proxvc/mixin/client/SoundEngineMixer.java
Outdated
Show resolved
Hide resolved
|
Thanks for all the feedback and consideration, this is all a learning experience for me so all the feedback only helps. I'll start chipping away at some of the issues mainly just working to cleanup the big stuff e.g. my complete disregard of the build.gradle file and AccessWideners loll. You can tell how little I understood going in to this so like I said feedback only helps. |
… socket implementation, reformating, fixed some naming oversights, optimized texture loading.
|
Ok so, I cleaned it all up and moved all the other features onto a different branch for a future PR or whatever I end up doing with them and I tried to keep in the scope of just a port for now. The only additional feature not in scope is the isTalking() method along with the icon associated with it. |
…logs, and removed redundant functions from port. Reformatting for parity. Removed left over mixer lang definition.
7.3 Port fr this timeHeres a summary of the changes I'm now attempting to merge after removing all the random things I added. i def have too much time on my hands having fun doing my first pr. Port
Additional implements
|
5Avian
left a comment
There was a problem hiding this comment.
Awesome! Seems like a solid PR. I pointed out some minor stuff that should probably be fixed to keep everything consistent, and a possible issue with the volume being set to 0. But great stuff, it's really nice to see that people like this enough to work on it themselves. And it's super useful, since I can't do any developing on my own right now.
Also, I tried launching this, but found this error. I assume this has to do with the build.gradle? Maybe BTA removed LWJGL from its' binary so that it needs to be added manually? The template project probably has the way to solve it.
Minecraft has crashed!
----------------------
Minecraft has stopped running because it encountered a problem.
If you wish to report this, please copy this entire text and post it on the support portal at https://bugs.betterthanadventure.net/
Please include a description of what you did when the error occured.
System Information
------------------
Generated 7/13/25, 11:48 AM
Minecraft: Better than Adventure! 7.3_01
OS: Windows 11 (amd64) version 10.0
Java: 21.0.5, Eclipse Adoptium
VM: OpenJDK 64-Bit Server VM (mixed mode, sharing), Eclipse Adoptium
LWJGL: 3.3.4-snapshot
[failed to get system properties (java.lang.NoClassDefFoundError: Could not initialize class org.lwjgl.system.Library)]
Error Information
------------------
java.lang.UnsatisfiedLinkError: Failed to locate library: lwjgl.dll
at org.lwjgl.system.Library.loadSystem(Library.java:174)
at org.lwjgl.system.Library.loadSystem(Library.java:64)
at org.lwjgl.system.Library.<clinit>(Library.java:52)
at org.lwjgl.glfw.GLFW.<clinit>(GLFW.java:30)
at net.minecraft.client.render.window.GameWindowGLFW.init(GameWindowGLFW.java:70)
at net.minecraft.client.Minecraft.startGame(Minecraft.java:481)
at net.minecraft.client.Minecraft.run(Minecraft.java:979)
at net.minecraft.client.Minecraft.startMainThread(Minecraft.java:2681)
at net.minecraft.client.Minecraft.main(Minecraft.java:2787)
at net.fabricmc.loader.impl.game.minecraft.MinecraftGameProvider.launch(MinecraftGameProvider.java:475)
at net.fabricmc.loader.impl.launch.knot.Knot.launch(Knot.java:74)
at net.fabricmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:23)
at net.fabricmc.devlaunchinjector.Main.main(Main.java:86)
After these issues are fixed, I'd be ready to merge and release :D
src/main/java/fiveavian/proxvc/gui/MicrophoneListComponent.java
Outdated
Show resolved
Hide resolved
|
Yeah seems the build.gradle needed to be updated. Only reason it was compiling for me was I had all the dependencies in my cache. |
|
Will this ever be merged and a new build released? I want to use this. |
|
Perhaps some day. I likely won't be spending any time on programming related things in the near future, so if you wanna use this version, I recommend building it yourself. The MIT license that this project is under allows you to do basically anything with the project, including (forking it and) releasing it under your name on platforms like CurseForge and Modrinth, as long as you include the exact license text for this project in source code and releases. |
There’s a release of this under my fork of the project. |
Fully Ported ✅
I haven't been able to bug test on any other platforms but, using 2 instances of BTA 7.3_04 on my pc running windows 11 it works without error (I can hear my self).
I ported the project environment to 7.3, fixed up the code so it doesn't crash on 7.3 apis, and added some client sided features like the volume mixer and mic indicator.
This is my first time coding in java and my first time contributing on here so I think it reaaallly does need some feedback.