# World resource system for Adventure Game/RPG

I'm beginning the long process of developing an Adventure Game/RPG and I was hoping to get some review on a resource system I'm coming up with. How it works is, a resource is generic to every resource in the world. Each resource will have a name, a random yield, and a random quality. So I've created an interface for the generic properties and function each resource will have.

Interface

public interface IResource{
string Name {get;set;}
string Quality {get;set;}
int Yield{get;set;}
string ConvertQualityToString();
int GetResourceYield();
}


Then I implement the interface in an actual resource base class.

Implementation

public class Resource : MonoBehaviour, IResource{
public string Name { get; set; }
public string Quality { get; set; }
public int Yield { get; set; }

public string ConvertQualityToString(){
var val = Random.Range(0, 3);
string quality;
switch (val){
case 1:
quality = "Good";
break;
case 2:
quality = "Great";
break;
default:
quality = "Poor";
break;
}
return quality;
}

public int GetResourceYield(){
var val = Random.Range(1, 25);
return val;
}
}


And finally, I create the specific resource itself and attach it to my game object in the inspector.

Resource Class

public class Granite : Resource{
void Start(){
Name = "Granite";
Quality = ConvertQualityToString();
Yield = GetResourceYield();
}
}


Is this the proper way to work this system? Obviously, the system actually does work as when I log the values on startup, Yield and Quality are randomized and the Name for each resource object I create is correct. Is there a better way to do this? Are there things I should change? Thanks in advance.

• All IResource properties have public setters. Are you absolutely certain that other code should be able to modify those? I'd recommend making these read-only unless you have specific reasons not to.
• ConvertQualityToString and GetResourceYield look like internal helper methods. Making them part of the IResource interface creates a very confusing API: what's the difference between resource.Yield and resource.GetResourceYield()? With the way the code is structured, I'd expect these methods to be protected.
• Are you sure that string is a good type for Quality? If you've got a fixed number of qualities an enum is a more strongly-typed and descriptive choice. And if different qualities have different characteristics then perhaps a Quality class makes more sense. Try to avoid writing 'stringly-typed' code.
• ConvertQualityToString gives a 33% chance for each quality. Likewise, GetResourceYield always produces yields between 1 and 25. Are you sure that's applicable to all resources and in all circumstances? Should quality and yield even be determined by a Resource instance itself? What if, for example, some enemies should have a higher chance of dropping high-quality resources? Or what if you want to provide player upgrades that increase the average resource yield?
• What's the point of that Granite class? There's no custom logic in there, and the resource name itself is sufficient to distinguish between different resources.
• In ConvertQualityToString, there's no need for that local quality variable: you can let each case return a value directly (which also allows you to remove those break statements).
• ConvertQualityToString is a strange name: it doesn't really convert anything. Inaccurate and confusing names tend to make code more difficult to work with.
• Thanks for the notes. There is going to be more to the system, for example, resources will be havestable, which is why I gave the properties public setters. The Granite class will have some logic in it, for example when a player is mining the granite resource, that's what will change the Yield of that specific resource. As far as the ConvertQualityToString and GetResourceYield methods go, should I just break them out into a helper class? Thanks again for the advice. – Robert Oct 19 '18 at 15:10
• Harvesting only requires Yield to be public (or perhaps better, a public Harvest method). And unless the mining mechanism for granite is really different from other resources, you don't need a separate class for it. As for those methods, I'd probably leave them where they are, but I'd make them static. Also, it looks like IResourceDeposit would be a more accurate name for how you're using it. – Pieter Witvoet Oct 20 '18 at 11:46