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.

Sign up for GitHub

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

Changes in fov resolution #3071

Closed
wants to merge 1 commit into from
Closed

Changes in fov resolution #3071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions engine/src/main/java/org/terasology/logic/players/LocalPlayerSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public class LocalPlayerSystem extends BaseComponentSystem implements UpdateSubs
private LocalPlayer localPlayer;
@In
private WorldProvider worldProvider;
private Camera playerCamera;
public Camera playerCamera;
Copy link
Contributor

@emanuele3d emanuele3d Aug 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Why this change?

From the PR alone it seems unnecessary. If it's needed elsewhere it's good practice to comment your own PR about changes that might be puzzling. 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The camera present here is needed in another system. Thats why made is public, though a getter method would have been better. But the camera is to be refactored in coming months so i just did it public.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about you get it from the WorldRenderer then? It's the object that instantiates the camera in the first place and you can find it on the Context/CoreRegistry. No point in going through intermediaries if it can be avoided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that would be better. But @In annotation doesn't work on WorldRendererImpl. I think i will need to get it directly from context. Will make the changes today :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Getting it from the Context is even better. In theory, having spoken with Immortius some time ago, we'd like to move away from annotation-based injection and use the context (and sub-contexts) more widely.

That been said:

@In 
private WorldRenderer worldRenderer;

doesn't work? It should.

@In
private MovementDebugCommands movementDebugCommands;
@In
Expand All @@ -109,6 +109,8 @@ public class LocalPlayerSystem extends BaseComponentSystem implements UpdateSubs
private Config config;
@In
private InputSystem inputSystem;

private MovementMode lastMode;

private BindsConfig bindsConfig;
private float bobFactor;
Expand Down Expand Up @@ -489,10 +491,13 @@ private void updateCamera(CharacterMovementComponent charMovementComp, Vector3f
}
}

if (charMovementComp.mode == MovementMode.GHOSTING) {
playerCamera.extendFov(24);
} else {
playerCamera.resetFov();
if(charMovementComp.mode != lastMode){
if (charMovementComp.mode == MovementMode.GHOSTING) {
Copy link
Contributor

Choose a reason for hiding this comment

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

And by the way, what's the GHOSTING mode?

Copy link
Member

Choose a reason for hiding this comment

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

IIRC it is literally the ghost movement mode you can enter via the game console. It changes the camera a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, cervator is right. Previously it used to change the camera as per the mode in every loop. Now it changes the camera only when the mode is changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, thank you. I can see where the "ghost" term comes from - going through walls.

I wish we could find a term that was a bit more explicit in its meaning. But perhaps just a short comment beside the constant in MovementMode would help. I.e.

GHOSTING(0f, 4f, false, false, false, 5f, true), // enables flying and going through solid surfaces

I checked and it seems the only mode that is somewhat ambiguous.

Copy link
Contributor

@OvermindDL1 OvermindDL1 Aug 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

noclip is the traditional meaning for a 'ghost' (travel through things) mode in engines, if you are looking for a more specific name. Where flying is the traditional term for being able to fly unbounded. And both combined is usually not given its own name though some things of recent use spectator mode for that, though that one is often non-interactable.

Copy link
Contributor

Choose a reason for hiding this comment

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

noclip sounds like a very technical term that only people familiar with the opengl meaning of clipping might understand.

It's been used in cheat codes since back to Doom days idnoclip. ^.^

Copy link
Member

Choose a reason for hiding this comment

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

Ahh good ole Doom days :-)

But yeah noclip may make sense if you're a seasoned gamer, but just the term on its own always stuck out to me as weird.

We have a separate flight command that works without the noclip or camera tweaks. Could ghost be something else? Prolly. spectator I would think of as something else (having no body)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, if we are brainstorming about this:

immaterial mode, intangible mode, wisp mode, incorporeal mode... oh... I like incorporeal.. let me find some synonyms... bodyless, disembodied, discarnate (yuck!), spectral, spiritlike, ethereal, transcendent, supernatural, nonphysical...

I guess incorporeal is the one I like the best, because I feel most people instinctively would understand that something incorporeal can go through walls. That been said, incorporeal might twist the tongue of some teenager and spiritlike is a more dumbed down contender. Except that some spiritlike modes, i.e. in World of Warcraft, still abide to -some- of the laws of physics, i.e. they must follow the landscape rather than fly through it.

So. Still vote for incorporeal mode. It seems very clear, unambiguos to me. But sticking to ghost (plus comment) or switching to spectator (or maybe observer? watcher? naah) would be fine too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was also thinking that terms such as spectator, observer and watcher might have their use in specific game modes.

Copy link
Member

Choose a reason for hiding this comment

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

I was also thinking that terms such as spectator, observer and watcher might have their use in specific game modes.

Yep, agreed.

incorporeal is a cool term, even if a bit long. wisp is short and kinda neat, but pretty close to ghost and not obvious. Maybe we should drop in a TODO or issue to consider the rename as part of the movement refactor since that could involve tweaking at the movement mode. Maybe of interest to @portokaliu

playerCamera.extendFov(24);
} else {
playerCamera.resetFov();
}
lastMode = charMovementComp.mode;
}
}

Expand Down
2 changes: 1 addition & 1 deletion engine/src/main/java/org/terasology/physics/components/TriggerComponent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
@ForceBlockActive
public class TriggerComponent implements Component {
@Replicate
public CollisionGroup collisionGroup = StandardCollisionGroup.SENSOR;
public CollisionGroup collisionGroup = StandardCollisionGroup.DEFAULT;

@Replicate
public List<CollisionGroup> detectGroups = Lists.<CollisionGroup>newArrayList(StandardCollisionGroup.DEFAULT);
Expand Down