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

Implemented zoom functionality #721

Closed
wants to merge 1 commit into from

Conversation

WyattTheSkid
Copy link

An OptiFine replacement needs to replace the zoom feature too.

@Dream-Master Dream-Master requested review from mitchej123 and a team November 4, 2024 18:53
Copy link
Contributor

Choose a reason for hiding this comment

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

jfc

proper one would be "Przybliżenie"

if (zoom) {
// Adjust zoom level with mouse wheel while holding zoom key
int dWheel = Mouse.getDWheel();
if (dWheel != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

This entire if can be replaced with zoomamt -= Integer.signum(dWheel);

import org.lwjgl.input.Keyboard;
import org.lwjgl.input.Mouse;

@Mod(modid = "skidzoom", name = "SkidZoom", version = "1.0")
Copy link
Member

Choose a reason for hiding this comment

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

please don't create another mod, just hook it up to angelica

private int zoomamt = -7;

@Mod.EventHandler
public void init(FMLInitializationEvent event) {
Copy link
Member

Choose a reason for hiding this comment

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

only register stuff on the client

@@ -530,7 +530,13 @@ public enum Mixins {
.addMixinClasses("mcpatcherforge.ctm_cc.MixinTextureMap")
),
//End from NotFine

//SkidZoom
SKIDZOOM(new Builder("zoom zoom")
Copy link
Member

@Alexdoru Alexdoru Nov 4, 2024

Choose a reason for hiding this comment

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

please remove all the word "skid" that you put everywhere, it means "stealing code" and that is not appropriate

Copy link
Author

Choose a reason for hiding this comment

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

I'm sorry I just seen all these. I'm not super familiar with the way that pull requests work and stuff so I missed all of these my apologies. And as for the word skid, its a copy pasta implementation of a mod I made for myself a while ago that was also called SkidZoom so I apologize for not seeing these messages, that is all. I will turn on notifications for any potential future pull requests.


//SkidZoom
SKIDZOOM(new Builder("zoom zoom")
.setSide(Side.CLIENT).setPhase(Phase.LATE)
Copy link
Member

Choose a reason for hiding this comment

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

if it's vanilla it should be Phase.EARLY

@Mod(modid = "skidzoom", name = "SkidZoom", version = "1.0")
public class SkidZoom {

public KeyBinding zoomKey;
Copy link
Member

Choose a reason for hiding this comment

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

doesn't need to be public


@Mixin(InventoryPlayer.class)
public abstract class InventoryPlayerMixin {
@Inject(method = "changeCurrentItem", at = @At("HEAD"), cancellable = true)
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that you wrap the int i = Mouse.getEventDWheel(); line in the Minecraft.runTick() method and make it return 0 when it's zooming

public class SkidZoom {

public KeyBinding zoomKey;
private static boolean zoom;
Copy link
Member

Choose a reason for hiding this comment

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

why is one field static and not the 2 others


@SubscribeEvent
public void ZoomyZoomZoom(FOVUpdateEvent e) {
if (zoomKey.getIsKeyPressed() || zoomKey.isPressed()) {
Copy link
Member

Choose a reason for hiding this comment

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

don't use isPressed(), only use getIsKeyPressed()

public void ZoomyZoomZoom(FOVUpdateEvent e) {
if (zoomKey.getIsKeyPressed() || zoomKey.isPressed()) {
zoom = true;
if (zoom) {
Copy link
Member

Choose a reason for hiding this comment

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

zoom = true followed by if(zoom) is this thing made by chat GPT ? and then you make us review it ?

Copy link
Member

@Alexdoru Alexdoru left a comment

Choose a reason for hiding this comment

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

also your implementation is broken, it will stayed zoomed for a bit if you unpress the key because the even you hook into isn't fired for every frame. You need to mixin into the rendering loop and modify the FOV in there

Also you should add a setting to have a "smooth zoom" and "instant zoom"

@Alexdoru
Copy link
Member

Alexdoru commented Nov 7, 2024

I'm gonna do my own

@Alexdoru Alexdoru closed this Nov 7, 2024
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.

4 participants