#using UnityEngine;public class UnitAdder...

1 messages · Page 1 of 1 (latest)

civic jasper
#

Instantiate takes a param for the parent

var unitObject = Instantiate(singleSlotUnit.GetUnitData.UnitPrefab, selectedSlots[i].transform.position, Quaternion.identity);
                unitObject.transform.SetParent(selectedSlots[i].transform);

Can just be

var unitObject = Instantiate(singleSlotUnit.GetUnitData.UnitPrefab, selectedSlots[i].transform.position, Quaternion.identity, selectedSlots[i].transform);
polar granite
#

oh, nice

#

thanks for involvement

civic jasper
#

If you spawn your prefabs slightly differently, you do not need to do the GetComponent<T> () here

selectedSlots[i].SetCurrentUnit(unitObject.GetComponent<Unit>());```

Read this:
https://bronsonzgeb.com/index.php/2021/05/01/unity-architecture-pattern-structured-prefabs/
polar granite
#

oh, interesting

#

so I dont have to reference the prefab

#

i can referece a component on that prefab

civic jasper
#

You reference the component you want from the parent transform on the prefab

polar granite
#

i tried doing it like

#
Instantiate(doubleSlotUnit.GetUnitData.UnitPrefab, selectedSlots[1].transform.position, Quaternion.identity) as Unit
#

but that didint work

#

i remember someone was doing it that was with the as keyword

civic jasper
#

I think because you've assigned the GameObject as the reference

#

You can't convert GameObject > Unit

polar granite
#

I did try with Unit aswell

#
Unit unit = Instantiate(singleSlotUnit.GetUnitData.UnitPrefab, selectedSlots[i].transform.position, Quaternion.identity) as Unit;
#

so the UnitPrefab should be as Unit, not GO?

civic jasper
#

shouldn't need the 'as Unit'

polar granite
#

is it a good practice?

#

naming the UnitPrefab not an actually prefab

civic jasper
#

name it what makes sense to you

#

it's the Unit component on the prefab

polar granite
#

oh, it worked

#

i also tried to extract the duplicate code to methods

#

but thats kinda hard

#

since the index are changing and stuff

civic jasper
#

I'd do this with the spawning

private Unit SpawnUnit(Unit unitToSpawn, Transform parent)
{
    return Instantiate(unitToSpawn, parent.position, Quaternion.identity, parent);
}```
polar granite
#

i have 4 similiar blocks in my class

#

do you have any idea for it?

civic jasper
#

var unitObject = SpawnUnit(singleSlotUnit, selectedSlots[i].transform);

polar granite
#

oh, I can even assign

#

current unit

#

in the SpawnUnit method you just made

civic jasper
#
if (selectedSlots[0].GetAdjacentSlots[0] == selectedSlots[1])
{
    var leftSlotUnitObject = SpawnUnit(doubleSlotUnit, selectedSlots[1].transform);
    var rightSlotUnitObject = SpawnUnit(doubleSlotUnit, selectedSlots[0].transform);

    selectedSlots[0].SetCurrentUnit(rightSlotUnitObject);
    selectedSlots[1].SetCurrentUnit(leftSlotUnitObject);

    selectedSlots[0].SetCanBeSelected(false);

    SlotSelector.Instance.ClearSelectedArray();
    return;
}
polar granite
#

yea that looks much cleaner

#

i

#

can I refactor it real quick and send you the new version?

civic jasper
#

You could even do this

if (selectedSlots[0].GetAdjacentSlots[0] == selectedSlots[1])
{
    selectedSlots[0].SetCurrentUnit(SpawnUnit(doubleSlotUnit, selectedSlots[1].transform););
    selectedSlots[1].SetCurrentUnit(SpawnUnit(doubleSlotUnit, selectedSlots[0].transform););

    selectedSlots[0].SetCanBeSelected(false);

    SlotSelector.Instance.ClearSelectedArray();
    return;
}
polar granite
#

i can call a method as a parameter?

#

gosh

civic jasper
#

The method returns the object the param requires

polar granite
#

oh, right

#

wait

civic jasper
#

Just depends if you like how readable that is

polar granite
#

it could be even simplier i think?

civic jasper
#

go refactor, test.. then re-share

polar granite
#

okay

#

line 85

#
Unit unit = Instantiate(doubleSlotUnit.GetUnitData.UnitPrefab, selectedSlots[i].transform.position, Quaternion.identity, selectedSlots[i].transform);
civic jasper
#

Unit unit = Instantiate(doubleSlotUnit, selectedSlots[i].transform.position, Quaternion.identity, selectedSlots[i].transform);

No?

polar granite
#

hmm

#

then i dont need the UnitSO

civic jasper
#

SO being ScriptableObject?

polar granite
#

yes

#

then i dont need both of this classes

#

but getting rid of Unit will cause some issues

civic jasper
#

Well, you'd still use the damage/ health from the SO

polar granite
#

yea, later on

#

so i dont need to have reference to the preafb in SO file

#

i can keep them (references) in UnitAdder

#

is that okay?

civic jasper
#

correct

polar granite
#

actually

#

im so happy you answered

#

can i resend you the refactored code?

civic jasper
#

I already said it's ok ;p

polar granite
#

here you go

#

looks much cleaner now

civic jasper
#

Yep, lovely

polar granite
#

also, do you think i should add more comments?

civic jasper
#

Good code doesn't need commenting

polar granite
#

is it good tho?

#

did you have problems reading it?

civic jasper
#

Not anymore

polar granite
#

xD

civic jasper
#

Is this a solo project?

polar granite
#

yes, I am trying to get a job in game dev but before i submit any CV im trying to learn making good code so im making some projects

#

what about thing like this?

    public void AddSlotToSelectedArray(Slot slotToAdd)
    {
        if(selectedSlots[0] != null && selectedSlots[1] != null)
        {
            selectedSlots[0].ChangeSlotState();
            selectedSlots[0] = slotToAdd;

            //Swap slots, so the first index is always the earliest selected one
            Slot tempSlot = selectedSlots[0];
            selectedSlots[0] = selectedSlots[1];
            selectedSlots[1] = tempSlot;

            selectedSlots[1].ChangeSlotState();
        }
        else if(selectedSlots[0] != null && selectedSlots[1] == null)
        {
            selectedSlots[1] = slotToAdd;
            selectedSlots[1].ChangeSlotState();
        }
        else if(selectedSlots[0] == null && selectedSlots[1] == null)
        {
            selectedSlots[0] = slotToAdd;
            selectedSlots[0].ChangeSlotState();
        }
    }
#

is it worth refactoring?

civic jasper
#

Great - having finished projects in a portfolio is a must have

polar granite
#

i can make another method called like AddSlot

#

oh nvm actually

#

i think its fine its not that complex

civic jasper
#

I got my first junior role with lots of repeated code

polar granite
#

as unit dev?

civic jasper
#

Yep

polar granite
#

nice, where are you at now?

civic jasper
#

You're already way ahead of where I was

polar granite
#

thanks thats nice to hear

#

my problems are mostly architecture stuff

#

like communication between classes

#

which pattern should I use etc

#

but im getting there 😄

civic jasper
#

Actions are my go to

polar granite
#

i tried Actions in this project

#

but I didint like it

#

or I probably did it wrong

#

like.. my slot has OnPointerClick method from the interface

#

and I fired an event from that method

#

but I didint know how to make SlotSelector listen to this event correctly

#

or should it be the other way around? so SlotSelector is invoking the event and Slots are listening?

#

do you know any youtube channel that is well explaining patterns?

#

im literally using singleton everywhere

civic jasper
#
public class SomeClass : Singleton<SomeClass>
{
    public Action OnSomethingHappened;
    public Action<bool> OnSomethingElse;

    private void Start()
    {
        OnSomethingHappened?.Invoke();
        OnSomethingElse?.Invoke(true);
    }
}


public class SomeOtherClass : MonoBehaviour
{
    private void Awake()
    {
        SomeClass.Instance.OnSomethingHappened += DoThing;
        SomeClass.Instance.OnSomethingElse += DoOtherThing;
    }

    private void OnDestroy()
    {
        SomeClass.Instance.OnSomethingHappened -= DoThing;
        SomeClass.Instance.OnSomethingElse -= DoOtherThing;
    }

    private void DoThing()
    { 
        // whatever
    }

    private vod DoOtherThing(bool someBool)
    {
        // whatever
    }
}```
#

I used a Singleton here for ease of demostration, but don't use a tonne of Singletons

polar granite
#

okay so live example, how would you do that? without code just short explanation
I have SlotSelector.cs that has an array with selected slots that is of size [2], you can have only 2 selected slots at 1 time.
Now, I am trying to make a SlotSwapper.cs that will swap units between slots, so if you choose 2 slots and click the SWAP button, they will swap. I can make some public method like CanSwap() in SlotSelector, since its a singleton, but I dont think that SlotSelector should know anytyhing about swapping

#

but i dont know how can i make this work without making CanSwap() in SlotSelector()

#

i'd ideally want to have this method in SlotSwapper and then if canswap, handle rest of the logic

civic jasper
#

What's the purpose of CanSwap() ?

polar granite
#

well, sometimes you cannot swap units

#

like you cannot swap X1 unit with X2 unit

#

and some other things

#

like space relevant etc

#

X2 untis take 2 slots, X1 untis take 1 slot

#

so i have to actually know if I can swap

#

if ther will be enought slots for X2 unti after swapping etc

civic jasper
#

SlotSwapper would need to know the data to be able to determine itself if CanSwap

#

this scenario doesn't sound like it needs actions, but, SlotSelector would have some properties (2 getters?) and SlotSwapper would access them to do its own CanSwap()

polar granite
#

yes, i can gather the data from the SlotSelector because its singletone

#

i probably asked the question wrong

#

how could I do this without singletone?

#

and what if, for example CanSwap would require data from SlotSelector and a Unit idk, just an example

#

Unit is not a singletone

#

probably a wrong example, because I can get a unti from a slot, bnut you know what i mean

#

lets assume i cannot

civic jasper
#

you'd still need a reference to SlotSelector

private SlotSelector slotSelector;

private void Awake()
{
   GetRequiredComponents();
}

private void GetRequiredComponents()
{
    slotSelector = FindObjectOfType<SlotSelector>();
}
polar granite
#

ohh, FindObjectOfType? really?

#

im avoiding those methods as hell

#

when I had 3 month practice at a gamedev company

civic jasper
#

It's fine as long as it's not being called every frame

polar granite
#

they told me to never use them

#

either in udpate and start/awake etc

#

so i didint

#

what if i have a lot of files in the hierarchy?

#

any performance issue?

civic jasper
#

the other option is to expose the field in the inspector with [SerializeField] and assign it manually

#

Never noticed a performance issue with FindObjectOfType

polar granite
#

so manually assigning wouldnt work

civic jasper
#

You're manually spawning the SlotSelector and SlotSwapper ?

polar granite
#

no no

#

just an example, my english sucks probably

#

if i spawn things in the runtime

civic jasper
#

Your English is fine 🙂

polar granite
#

manually assignment wouldnt work

civic jasper
#

If you spawn things at runtime , the thing that spawns them can already have the relevant references, and assign them on spawn

polar granite
#

so you're saying that I shouldn't be afraid using FindObjectOfType<T>

#

obviously not calling every frame

#

but just when needed

civic jasper
#

If you're spawning a thousand things, then FindObjectOfType<T> is probably gonna be an issue, even in Awake/ Start.. but one or two.. no it's fine

polar granite
#

im still afraid how would potentional CTO reviewinng my recrutiment task look at it

#

its so confusing since there are a lot of "ways" to code things

#

and if the CTO doesnt like my "way" but it works and it's fine

#

then im basically fked

civic jasper
#

I assume you're going for junior level roles?

polar granite
#

yes

civic jasper
#

Any interviewer reviewing your code isn't going to dismiss you for 1 thing

polar granite
#

learning till end of summer then im looking for a job

civic jasper
#

Anyone who dismisses you for doing things differently, instead of seeing a potential learning opportunity for you.. is probably someone you don't wanna work with anyway - depending on country and work culture I guess

#

If you're not sure on FOT, profile it when you use it

polar granite
#

im from Poland, in my opinion we have some great studious over here like CDP, Techland etc

#

so the gamedev industry is nothing new here

civic jasper
#

You can back up your use of it then "I profiled the use of it when I added it, and it gave no performance issues"

polar granite
#

actually I have to go now pick up my gf from the work

#

thanks for the time Sir

#

cleared my mind

civic jasper
#

np - laters

polar granite
#

hey @civic jasper , got a question about architecture, so:

just to visualize things, this is my gameplay screen

#

ADD X1, ADD X2 - add 1 or 2 slot units to the selected slots
DELETE - deletes the units on the selected slots if there is any to delete

now as I mentioned previously, I am trying to make a SWAP mechanic that swaps the selected units and we already discussed it so I got it covered. The problem is that I want to add something like - pseudocode:

when 2 slots are selected the SWAP button is either clickable or unclickable (lets say it also changes color)
and im having issues implementing this from the architecture side.

my SlotSwapper.cs has a TrySwap() public method that is assigned to a SWAP button then it checks if it can swap etc etc, but I want to dynamically set the SWAP button to clickable or unclickable as soon as 2 slots are selected. How would I do that?

#

I hope you understand

#

===
TLDR: everytime I select a slot and I have two slocts selected I need to make the SWAP button clickable/unclickable

#

Is making a reference to SlotSwapper.cs in the SlotSelector.cs, then in the Update() in the SlotSelector.cs do check if there are 2 slots selected then do a check if they are swappable then turn on/off the button a good idea?

#

or is it better to use Actions and invoke them everytime a slot is selected?

#

I wanted to use actions at first, but I didint use them anywhere in my project and I think it'd look bad for the potential recruiter if he sees it, because he might think like: "Why didn't you use Actions earlier for deleting/adding the units? It'd be easier etc" but in the other hand, the recruiter might see that I know some stuff about events aswell so im confused

#

OnPointerClick method from the interface is on Slot.cs which makes things harder, if it'd be onSlotSelector.cs it'd be much easier but I cannot change it now

civic jasper
#

Don't use Update() for this, it doesn't need to be running every frame

#

Whichever class deals with a slot being selected, should be the one setting button.interactable

polar granite
#

okay

#

the thing is

#

that the class that is dealing with slot beign selected its slot.cs itself

#

Slot.cs has IPointerClick method

#

making a reference to a button in Slot.cs doesnt make much sense to me