Get and Set for member public vars not working

Hello all :slight_smile:

I’m trying to use Get and Set implicit methods and nothing appears to work…


public bool enable=true;
public bool Enable
{
   get { return enable; }
   set { Debug.Log("balors?");if(!value) DoSomething(); enable = value; }
}

It appears the ‘Enable’ have nothing to do with the ‘enable’…

Could anyone please explain me what i’m missing ?

Thanks a lot and happy unitying !

The term is “property” with get and set “accessors.”

What is the exact issue you’re having?

ow… i did not mention my real problem. sorry.

It appears that the accessors methods are never called when accessing the ‘enable’ property.

When setting it from script or inspector or anim, neither get nor set are called.

This is the way things work, unfortunately. Serialization and editor interfaces operate on fields, not properties. You could use a custom editor that specifically modifies the property in code, but it gets finicky.

Oh ok so, get and set are not welcome to be used in unity ? :confused:

Isn’t there any way U3D offers for monitoring changes on a member variable ?

Thanks for your answer :slight_smile:

Unity has a separate Unity Properties API which does support using properties. In general though, as annoying as it is, typical serialization-based hookup etc. does not use properties. You can of course elect to only publicly expose properties in your code, but Unity’s systems will still use field-based mechanisms.

Thanks for your answer.

Then i guess my only choice is to poll the bool change in an Update().
An ugly solution but hopefully it will be the only one in my whole app :wink:

Thanks @Spy-Master :slight_smile: Have a great day !

Assuming this is only for Editor-time use you could always use OnValidate()

If it is for runtime, just route everything to use your .Enable property and make .enable private. That’s considered a “normal” method, but as noted above, doesn’t help you for Unity’s serialization.

Beware also that you’re dangerously close to the .enabled property in the Behaviour class, setting yourself up for really hard-to-find easy-to-make bugs in the future. Perhaps you’re just showing it as an example field…

yes, there are many solutions in deitor with OnValidate or even variable polling at OnChange in custom editor or custom inspector ( and at this time, performance is not an issue ).

But unfortunately i needed this at runtime :confused:

The only way i found is polling the bool value change in an Update… i hate this but the class doing this is unique^^

The second solution i could use if i had to poll multiple vars would be to create a static referencing the vars to be monitored and scanning all of them in a unique Update.

Thanks for your answer :slight_smile:

And happy unitying !

I’m not seeing the problem other than you are adding game logic to a setter which is a very bad idea. As the lead dev I would reject your pull request and ask that you follow standard practices.

If you care to explain what is the “polling” you need to do? Also keep in mind most if not all of the time if you have a “unique situation” that nobody has encountered using C#, Unity or in game development you could be creating your own issues.

I also reject all crappy things but when there are no better solution, they are an acceptable compromise.

Here’s the code needing the bool polling:

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class InWaterObserverChanger : MonoBehaviour
{

public bool enable=true;
bool prevEnable=false;

bool InWater=false; // Is player in water ( true ) or not ( false )



public float playerSpeedInWater = -1.0f;
public float playerJumpPowerInWater = 1.0f;
public float playerRunFactorInWater = 1.0f;
public float gravityFactorInWater = 1.0f;



   void Update()
   {
      // Monitor the enable flag
      
      if(!enable && prevEnable)
      {
         GlobalMessenger.BroadcastMsg("set_walk_speed",-1.0f);
         GlobalMessenger.BroadcastMsg("SetJumpPowerFactor",1.0f);
         GlobalMessenger.BroadcastMsg("SetGravityFactor",1.0f);
         GlobalMessenger.BroadcastMsg("SetRunFactor",1.0f);
         
         print("water force exit");
      }
      else 
         if(enable && !prevEnable && InWater)
         {
            GlobalMessenger.BroadcastMsg("set_walk_speed",playerSpeedInWater);
            GlobalMessenger.BroadcastMsg("SetJumpPowerFactor",playerJumpPowerInWater);
            GlobalMessenger.BroadcastMsg("SetGravityFactor",gravityFactorInWater);
            GlobalMessenger.BroadcastMsg("SetRunFactor",playerRunFactorInWater);
            
            print("water force enter");
         }
      
      prevEnable = enable;
   }



   void OnTriggerEnter()
   {
      InWater=true;
      
      if(!enable) return;

      GlobalMessenger.BroadcastMsg("set_walk_speed",playerSpeedInWater);
      GlobalMessenger.BroadcastMsg("SetJumpPowerFactor",playerJumpPowerInWater);
      GlobalMessenger.BroadcastMsg("SetGravityFactor",gravityFactorInWater);
      GlobalMessenger.BroadcastMsg("SetRunFactor",playerRunFactorInWater);
      
//      print("water enter");
   }   

   void OnTriggerExit()
   {
      InWater=false;
      
      if(!enable) return;

      GlobalMessenger.BroadcastMsg("set_walk_speed",-1.0f);
      GlobalMessenger.BroadcastMsg("SetJumpPowerFactor",1.0f);
      GlobalMessenger.BroadcastMsg("SetGravityFactor",1.0f);
      GlobalMessenger.BroadcastMsg("SetRunFactor",1.0f);
      
//      print("water exit");
   }   

}

This changes some vars according to player in or out water, knowing that water ( high or low level changed by an anim ) influence changes whenever the player is in water or out water…

This scripts works perfectly though the ‘Update()’ solution is crappy. A good compromise would be a 1 second period coroutine replacing the update…

If you have advice for better solution @tleylan i’d be honored i read it :wink:

Thanks and happy unitying !

I’m not sure you should be honored but I appreciate the sentiment. It’s just an old programmer helping other (probably younger) programmers :slight_smile:

There are almost no “crappy but no better solutions”. Some I’m sure but they are rare and are typically introduced when a platform doesn’t implement (or expose) a better solution. Not passing parameters to a custom event in VRChat for instance.

Nothing I see in InWaterObserverChanger suggests that a setter is required to process anything. Keep in mind that getters and setters are what is known as “syntactic sugar”. I like the sugar but they don’t expose any new functionality.

Quick review of this code reveals that you have 4 public “InWater” floats. These are a) constants and should be marked as such and b) you do not seem to have the equivalent “NotInWater”. I’m staring at the values and they seem to be identical. In any case you should not reference them as -1.0f in half the cases.

You’ve named one thing “set_walk_speed” and 3 others with a pattern that looks like “SetJumpPowerFactor”. Why not be consistent and use SetWalkSpeed?

Any time you see sets of lines that are almost identical consider a function that will reduce the exposure to bugs and probably increase testability. So the 4 GlobalMessenger.BroadcastMsg lines surely can be a private method. If you pass inWater as a parameter it should be able to handle the setting in either case.

I don’t see how enable gets set but you can make it private and and add a public SetEnable() method. Anything that would set the property would now just call the method. That method can set the property and execute whatever was needed.

A good compromise would be a 1 second period coroutine

Definitely a bad compromise and I would again reject your pull request. :slight_smile:

As that all gets cleaned up consider “as a rule” to not add code directly to an event. Your OnTriggerEnter and OnTriggerExit (again) do almost identical work with the only variant being a true or false value.

Call a method like ProcessTrigger(bool inWater) and call it from those events with the value you need.

Finally (and I know I’m pushing a limit here)

Consider not commenting the obvious.

bool InWater=false; // Is player in water ( true ) or not ( false )

The field is named InWater, if it isn’t clear enough then name it playerInWater. You don’t have comments on the other bools like what prevEnable is used for…

And it is good idea to explicitly add scope so private bool inWater is better. Yes it is private but there is nothing lost by explicitly writing it and you might question yourself whether something needs to be public or private as you are typing it.

And again, don’t add a 1 second period coroutine. It won’t make it through the review process :slight_smile:

Thanks for you long answer :smiley:

The enable flag is set from an animation.

The 4 floats set to their default values are exposed in inspector so that they can be changed to modify player behaviour in water or in swamps, etc…
The defaut values ( with -1 for speed in water ) is sent to reset the player state machine.

I agree for the set_walk_speed wich is a method of a class that is 14 years old and have evolved through various project. Sometimes a complete and clean rework is needed but not always done because of lack of time :stuck_out_tongue:

This is what i’ll do. It will make process less obfuscated :wink:

Thanks and happy unityin @tleylan !