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

Markervision: Generalize for all placeable entities #1423

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ZenBre4ker
Copy link
Contributor

This enables markervision for all placeable entities, by initializing and removing it in the baseclass.
To overwrite, change icons or add additional infos, you can use the ent specific functions.

This shall also be the start of remote triggerable entities, that make use of the markervision system, by enabling remote configuration when a markervision enabled entity is in line of sight.

@ZenBre4ker ZenBre4ker added the type/enhancement Enhancement or simple change to existing functionality label Feb 18, 2024
@ZenBre4ker ZenBre4ker marked this pull request as draft February 18, 2024 15:00
@@ -14,6 +14,10 @@ ENT.isDestructible = true

ENT.pickupWeaponClass = nil

-- MarkerVision related
ENT.iconMaterial = Material("vgui/ttt/tid/tid_big_role_not_known")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to do a missing icons icon.

@@ -12,6 +12,7 @@ DEFINE_BASECLASS("ttt_base_placeable")
if CLIENT then
ENT.Icon = "vgui/ttt/icon_health"
ENT.PrintName = "hstation_name"
ENT.iconMaterial = Material("vgui/ttt/icon_health")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

better health icon

if CLIENT then
ENT.Icon = "vgui/ttt/icon_decoy"
ENT.PrintName = "decoy_name"
ENT.iconMaterial = Material("vgui/ttt/icon_decoy")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

better decoy icon

@TimGoll
Copy link
Member

TimGoll commented Feb 18, 2024

One thing I'm not a fan of is the .iconMaterial. For two reasons:

  1. The .iconMaterial for roles and weapons is autogenerated
  2. The .Icon variable also has a different icon then the .iconMaterial which would be really confusing

@@ -24,6 +28,11 @@ function ENT:Initialize()

if SERVER then
self:PrecacheGibs()

local mvObject = self:AddMarkerVision("ttt_base_placeable_owner")
mvObject:SetOwner(self:GetOriginator())
Copy link
Member

Choose a reason for hiding this comment

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

not a fan that the owner is hard set to the originator here, the owner can also be a role or a team

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For what is the owner relevant anyways? I just hardcoded it as I saw no benefit to it being customizable. Normally you place it you own it.

Copy link
Member

Choose a reason for hiding this comment

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

The heroes crystal, that also uses this base, has the owner set to ROLE_SUPERVILLAIN. As any supervillain can see it, but a supervillain changing their role, will lose access to the marker vision

end

function ENT:GetMarkerVisionColors(mvData)
return { COLOR_WHITE }
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 this a color table? Is this one color for each icon? One for each line of text?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see this is for the icon, this is a bit confusing, because the text can have colors as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know. Will adjust.

@ZenBre4ker
Copy link
Contributor Author

Tried to do your review comments. Would start documenting later and then try myself with the remote trigger.

@ZenBre4ker ZenBre4ker added the status/waiting Depends on another Pull Request to be merged before label Feb 27, 2024
@ZenBre4ker
Copy link
Contributor Author

This is finished from my side, if Tim supplies the icons, it should be ready again.
It has no hurry for me personally, as I wont rely for now on the placeable entity, but on markervision only.

@TimGoll
Copy link
Member

TimGoll commented Feb 28, 2024

This is finished from my side, if Tim supplies the icons, it should be ready again. It has no hurry for me personally, as I wont rely for now on the placeable entity, but on markervision only.

I'll try to add the icons this week!

@TimGoll
Copy link
Member

TimGoll commented Feb 28, 2024

I took a quick glimpse at the code and I think @saibotk has to give his opinion here on the structure. I'm not entirely sure if this complicates it too much or so, seems a bit arbitrary to me, hm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/waiting Depends on another Pull Request to be merged before type/enhancement Enhancement or simple change to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants