How can I properly use OOP in this situation

I recently decided that I was going to start to learn OOP and I’ve been practicing it for a little bit. I’m just not sure if I’ve been using the best practices and I’m confused on some things. To practice composition I have a Plane class and an Engine class. I made it so that when the player presses V a plane spawns and the player can spawn multiple planes. I want to make it so that when the player sits down and presses E the engine’s IsOn property turns to true. I just don’t know if how I can identify what plane the player is sitting in. Here’s my code:

Plane class:

local Engine = require(script.Parent.Engine)

local Plane = {}

Plane.__index = Plane

function Plane.new(model)
	return setmetatable({
		model = model,
		Engine = Engine.new()
	}, Plane)
end

function Plane:TurnOn()
	Engine:ChangeEngineState()
end

return Plane

Engine class:

local Engine = {}

Engine.__index = Engine

function Engine.new()
	return setmetatable({
		IsOn = false,
		Fuel = 100,
	}, Engine)
end

function Engine:ChangeEngineState()
	if self.IsOn == false then
		self.IsOn = true
	else
		self.IsOn = false
	end
end

function Engine:CheckFuelLevel()
	print("Fuel level is "..tostring(self.Fuel) .. "%")
end

function Engine:AddFuel(fuelToAdd)
	self.Fuel += fuelToAdd
end

while Engine.IsOn == true do
	wait(1)
	Engine:AddFuel(-1)
end

return Engine

Server script:

local Plane = require(script.Parent.Plane)

game.ReplicatedStorage.SpawnPlane.OnServerEvent:Connect(function(player)
	local airplaneModel = game.ReplicatedStorage.Airplane:Clone()
	airplaneModel.Name = player.Name
	
	Plane.new(airplaneModel)
	airplaneModel.Parent = workspace.Planes
	airplaneModel:SetPrimaryPartCFrame(player.Character.HumanoidRootPart.CFrame * CFrame.new(0,5,-10))
end)

Local script:

local player = game.Players.LocalPlayer
local char = player.Character or player.CharacterAdded:Wait()

local events = game.ReplicatedStorage.Events

local userInputService = game:GetService("UserInputService")

userInputService.InputBegan:Connect(function(input, gameProcessedEvent)
	if gameProcessedEvent then return end
	
	if input.KeyCode == Enum.KeyCode.V then
		events.SpawnPlane:FireServer()
	end
end)

I also don’t know if it is good practice to use a RemoteEvent to call the Plane.new() function. Is this the best way to call the function whenever the player presses V? Should I use a RemoteEvent to call a function for whenever a player presses a specific key? For example when the player presses E should I have a RemoteEvent calling the event to start the engine on the server? This seems like a tedious process if the plane requires a lot of input, it would also require a lot of RemoteEvents. Sorry if these seem like stupid questions I’m completely new to OOP.

2 Likes

The logic for setting up the plane model, as well as detecting when a player sits in the seat, should probably be handled by the Plane class. I’d change the constructor to something like this:

function Plane.new(modelTemplate)
    local self = setmetatable({}, Plane)
    
    self.model = modelTemplate:Clone()
    self.model.Parent = game.Workspace.Planes
    self.model:SetPrimaryPartCFrame ...
    ... any other model setup

    self.Engine = Engine.new()

    return self
end

Your TurnOn method also isn’t correct, it should probably be

function Plane:TurnOn()
	self.Engine:ChangeEngineState()
end

You can detect when a player sits in a seat with the ChildAdded event. Look for a weld called SeatWeld whose Part1 is the HRP of a character, then get the player from that character. Set up the listener to ChildAdded in the constructor of the Plane class.

No, you should do all the client-side verification to make sure you only send an event when the client thinks it makes sense. I.e. don’t send an event every time the player presses any key. Only send an event when they press V AND they’re close enough to a plane AND they’re not already in a vehicle AND they’re not dead AND they bought the requisite gamepass AND etc. …and then repeat all the same checks and probably more on the server because the client can’t be trusted.

You can do that, or you can have one RemoteEvent for anything related to controlling the plane, and then send a string as the first argument to FireServer to tell the server what kind of “command” the player is trying to send. You could even have just one RemoteEvent for all the systems in your entire game this way. I prefer having separate RemoteEvents with descriptive names for each “command” the player can send to the server.

It’s really not. Even for an extremely complicated plane with say 20 inputs, just put them in a plane-specific folder in Events. You just have to spam Ctrl+D 19 times, then find 20 good descriptive names for the things you want them to do. Compared to designing and writing the code it’s nothing, at most 5 minutes if you were doing them all at once, and you can do them one at a time as you’re programming.

1 Like

In the example you gave me you changed the plane.new() function to this:

function Plane.new(modelTemplate)
    local self = setmetatable({}, Plane)
    
    self.model = modelTemplate:Clone()
    self.model.Parent = game.Workspace.Planes
    self.model:SetPrimaryPartCFrame ...
    ... any other model setup

    self.Engine = Engine.new()

    return self
end

I set the plane’s model’s position in front of the player when the plane is initially spawned. How can I know what player spawned the plane in the modulescript so that I can set the position of the plane to in front of the player’s character?

Before the while loop try adding this:

function Engine:UpdateFuel ()
    if self.IsOn == true then
        self:AddFuel(-1)
    else
        -- Add fuel or something if you want
    end
end

Then instead of a while loop (because it is bad practice) do this:

local Count = 0
local UpdateDelay = 1 --// The rate at which you lose fuel

function Engine:UpdateLoop ()
    return game:GetService("RunService").Stepped:Connect(function(delta) 
        Count += delta
        
        if Count >= UpdateDelay then
            Count = 0
            self:UpdateFuel()
        end
    end)
end

Then just call :UpdateLoop to start the loop. This has wayyyy more benefits because you can set a variable to :UpdateLoop kinda like this:

local FuelLoop = Engine:UpdateLoop ()

And the benefit is that you can destroy/disconnect the loop at any point (and it just looks better and is way better organized!) Like this:

if Engine.Fuel <= 0 then --// Just a example
    FuelLoop:Disconnect()
end

Also the reason why the while loop is even worse is because if the engine turns off the loop will never start again thus infinite fuel. And because you cant attach the loop to the class so you can never change it outside the module script.

Also if you really want you can make it so if you have a gamepass or a perk you lose fuel at a lower rate by changing “UpdateDelay” to be a part of the class like this:

Engine.UpdateDelay = 1
Engine.Multiplier = 1 --// This will be your multiplier

And change the :UpdateLoop() method to this

function Engine:UpdateLoop ()
    local Delay --// Use a capital D to prevent a mix-up between the global "delay"
    
    return game:GetService("RunService").Stepped:Connect(function(delta) 
        Count += delta

        Delay = self.UpdateDelay * self.Multiplier
        
        if Count >= Delay then
            Count = 0
            self:UpdateFuel()
        end
    end)
end

Then have three methods controlling the multiplier:

Decrease delay:

function Engine:DecreaseDelay ()    
    self.Multiplier = 0.5
end

Increase delay:

function Engine:IncreaseDelay ()    
    self.Multiplier = 2
end

Reset:

function Engine:ResetDelay ()    
    self.Multiplier = 1
end

Hope this helps!

If that is something that will be done for every plane, then pass the spawning player to the constructor as another argument

I’m still confused on how I can detect what plane the player is sitting in. I know how to detect what plane model the player is sitting in but how do I know what plane to call methods on. For example in the server script I have this:

local plane = Plane.new(airplaneModel)

Every time the player presses V “plane” is assigned to the plane that just spawned and any methods I call will be called on that plane. So how can I make “plane” the plane that the player is sitting in?

You can implement a Get function and keep a table of all the planes you created in the game. I do it this way:
(Referencing the model is just one of the many ways you can do this. You can also set a unique ID to each plane created and check it that way. The unique ID method is the same process as the one below, just slightly more reliable)

--Table keeping track of all the planes generated with the Plane.New() method
local planesInGame = {}

--Getter function
local function getPlane(model)
    for _, plane in pairs (planesInGame) do
        if plane.model == model then
            return plane
        end
    end
    return false
end

--\\Example for how to create a new plane//--
local plane = Plane.new(airplaneModel)
table.insert(planesInGame, plane)

--\\Example of how to use the Getter function//--
local planeModel = --A reference to the actual plane model that the player is inside. You can do this through the seat or whatever is easiest. 
--but you need to reference the model 

local hitPlane = getPlane(planeModel)

if(hitPlane)then
    hitPlane:WhateverMethodYouWantToUseOnThePlane()
end

For some reason when I try to do this:

local Plane = PlaneClass.new(game.ReplicatedStorage.Airplane, player)
table.insert(planesInGame, Plane)
print(planesInGame)

It just prints “{ }” so nothing is being inserted into the table. Do you know why this is?

what does OOP mean. I’ve seen it in many areas wondered what it meant :confused:

Are you doing local planesInGame = {} ?
(Trivial but sometimes people forget)

Yup, here’s my entire serverscript:

local PlaneClass = require(script.Parent.PlaneClass)

local planesInGame = {}

local function getPlane(model)
	print(planesInGame)
	for _, plane in pairs (planesInGame) do
		if plane.model == model then
			return plane
		end
	end
	return nil
end

game.ReplicatedStorage.playerInput.OnServerEvent:Connect(function(player, input)
	if input == Enum.KeyCode.E then
		local seat = player.Character.Humanoid.SeatPart
		local model = seat.Parent
		
		local plane = getPlane(model)
		print(plane)
		if plane == nil then return end
		
		plane:TurnOn()
	end
	
	if input == Enum.KeyCode.V then
		local Plane = PlaneClass.new(game.ReplicatedStorage.Airplane, player)
		table.insert(planesInGame, Plane)
	end
end)

Is that code in a module script? And if so, whats the name of the module script?
Another could-be-issue is that your PlaneClass.new() method might not be returning anything. So the value of Plane is nil.

That was the issue, but now another issue has arose. This is my engine class:

local Engine = {}

Engine.__index = Engine

function Engine.new()
	local self = setmetatable({}, Engine)
	
	self.Fuel = 100;
	self.IsOn = false;
	self.IsBroken = false;
	return self
end

function Engine:TurnOn()
	print(self.IsOn)
end

return Engine

When I call the Engine:TurnOn() method it prints nil. Any idea why it does that?

Edit: Nevermind I found the issue