0
\$\begingroup\$

I am currently working on a small side project (as usual), and trying to build up from previous experiences.

I have the following elements in the game: 1) Region: which contain a "food" target that has to be met 2) Location: Each region has a few location, where crops can be sown 3) Crop: A variety of crops with different effects

I currently have a system where I have a Region monobehaviour script that only considers the production of food from the crops. The location monobehaviour, stores the current state of the location, namely what crop is currently on it and how much food is currently available. I also have all the food production logic on that script. The crop scripts are scriptable objects with different food production rates and water consumption values.

I run into a problem when I started to want to add other types of elements to the location, or rather, have crops that don't necessarily produce food but have another effect on the game.

I decided to build another system, where instead of having the logic that triggers the various methods on the location script (for example the ProduceFood(), SendFood(), ReceiveFood()) within the location script itself or via hard links from UI elements.

This new systems is centred on Action scriptableObjects that can be added to the Crop scriptableobjects. The action script contains an Act(Location loc) method as well as a SetUp(Location loc) and Disable(Location loc). I can now customise how and when the ProduceFood(), SendFood(), ReceiveFood(), etc methods on the location script get called. For example the ProduceAction will add the Act method to a delegate event on the location script that triggers when a global OnProduction event gets triggered. Similarly, my hard UI links now point to a method on the location script called UITriggerAction(int index) which triggers the Act method registered to a delegate array on the location script.

Now each time a change the crop at a location I call the Disable method on all the Actions of the current crop, then I replace the crop and call the SetUp method on it, which will either register the associated Act method to the array accessible from UI, or to some other custom event.

The Act methods would be simple and look like:

Act(Location loc)
{
    loc.Produce();
}

or

Act(Location loc)
{
    if(loc.HasProduced())
    {
        loc.SendFood();
    }
}

By doing it this way I keep the variables specific to the location on the location script as well as the logic on it. In addition I avoid any dependency between the current crop at a location and the rest of the game. For example I could have a method from the Region script, that would check all the currentFood values on the locations, and it would still work even if the crop at that location doesn't impact the currentFood value at that location.

int AvailableFood()
{
    int availableFood = 0;
    for (i = 0; i < loactions.Length; i++)
    {
        availableFood += loactions[i].currentFood;
    }
    return availableFood;
}

I just control the way in which I call those methods using a more versatile system.

My question is: Is this likely to lead to a very messy code on the location script where I will have all the variables and all the methods to do everything?

Do you have any suggestions on how I can improve that system?

Supporting code:

class Location : MonoBehaviour
{

    Region parentRegion;
    Crop currentCrop;
    public int currentFood;
    int maxFood;

    public delegate CropAction(Location loc);
    public CropAction[] actions = new CropAction[6] // 6 possible actions accessible from UI

    public void UITriggerAction(int index) { actions[index](this);  }

    public void Produce() { // Do stuff   }
    public void SendFood() { // Do stuff   }
    public void RecieveFood() { // Do stuff   }
    public void UseWater() { // Do stuff   }
    public bool HasProduced() { return currentFood > 0;  }

    public AddCrop(Crop crop)
    {
        loc.Produce();
    }
}    
\$\endgroup\$
  • 1
    \$\begingroup\$ It sounds like you have a solution that works for your needs. If you'd like open-ended feedback / critique / improvement suggestions on a working code solution, we have a dedicated StackExchange for that: Code Review. By contrast, here on GameDev, we focus on targeted problem-solving for features that aren't behaving the way you want yet. So, have you encountered a problem with this approach that's negatively impacting your development process or the behaviour in-game? \$\endgroup\$ – DMGregory Apr 5 at 10:20
  • \$\begingroup\$ Thank you for the comment. I wasn't sure in which of the two Stacks it should go... So if you think it should be move I will do that. I think I need to wait a bit before I can delete in from here. \$\endgroup\$ – Sorade Apr 5 at 10:48
0
\$\begingroup\$

DMGregory is right. I'll offer some advice anyways, but in the future use the Code Review page.

What you are doing is called Data Driven Design. Some classic examples are Warcraft, Starcraft (1 and 2), and DoTA. I believe DoTA has done a full writeup on their system you can google. The cons of data driven design, is you give up a lot of public/private scoping because you need to query information everywhere. You also end up with a lot of if/elseif/else statements or switch statements. Good design can mitigate it but you'll still have more than a more OOP approach. The main pro of DDD is that things are insanely iterable and flexible. Extending / Modifying your design is done more via editor tooling than coding. In a large project where you can't possible code everything and maintain it, this is a good solution. It's also great if you want to expose an Editor to your users (like DoTA, Starcraft, and Warcraft).

Something you can do to help with the public/private dilema, is instead of exposing variables on Location that only Action may need you create an ActionContext struct, and pass that whenever you call Act. This also lets you get information from outside Location (like say current game score) you can just add that int to the struct. Whoever is calling the crop's Act method will be responsible for gathering that information. If it's the Game class then that's easy, you call a method on the Location class and pass the context with the data the Game class should provide. That method on the Location class adds it's information to the context then calls the crop. The crop adds it's information then calls Act on the action. You may also want to pass the Location class itself for brevity, but if you don't want to expose certain information on the Location class, make a struct field and have it fill out the struct. For example let's say we have a public interface for the Location class, and some private variables as well. Maybe we need one of those private variables, say "int privateVarA". Then you can make your context struct take an instance of a Location class, and an int "privateVarA". Now you can expose that private value to the action without having to expose it on the Location class as a whole.

public struct ActionContext
{
  // Required
  public Game game;
  public Location loc;
  public Crop crop;
  // Friend Variables
  public float RemainingCropGrowthTime;
  public float GameDifficultyCurve;
}

public void Act(ActionContext context) {
  // Do stuff
}
```
\$\endgroup\$
  • \$\begingroup\$ Thank you for your answer. Would the variables in ActionContext be passed by reference though ? To make sure that the values on the origin classes (eg. location) are modified ? Currently to bypass this problem, I have created short methods on the Location class to return the private variables. I guess I could also use private setters and public getters on the location class as well ? \$\endgroup\$ – Sorade Apr 5 at 14:31
  • 1
    \$\begingroup\$ If you wanted to emulate c++ friend classes for setting/getting private variables, it can get complicated and the payoff isn't usually there. I'd recommend just passing Location by value and having contextual methods for manipulating it. Avoid methods like "SetVariableA" and instead methods like "AddFunds(int amount)" and "RemoveFunds(int amount)". Or instead of setting a boolean "SetIsBarren(bool)" have a function "MakeBarren()" and "Fertalize()". Define your methods on how they impact the class, not the variable. \$\endgroup\$ – gjh33 Apr 5 at 15:03
  • \$\begingroup\$ This better defines the public api and prevents other classes from manipulating variables all willy nilly in ways you don't want. Your current class does this well. The big reason for using Context pattern is it lets you pass information outside one class' defined API if you need to. \$\endgroup\$ – gjh33 Apr 5 at 15:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.