Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linkage reworks #419

Merged
merged 12 commits into from
Aug 30, 2024
Merged

Conversation

LengthenedGradient
Copy link
Contributor

Introduces changes to how links are represented in the code. Backwards compatibility has been maintained, so RegisterClassLink/RegisterClassUnlink calls made previously still work. RegisterNewLink was replaced GetLinkDataSafe with to safely initialize tables, where each RegisterClass<something> function uses this internally. It can also be used to retrieve information about a link, e.g. GetLinkDataSafe("Class1","Class2").Link. This was done for future proofing reasons, as its now much easier to add new properties to links.

GetClassLink and GetClassUnlink were removed as you can do the same thing from the example above.

RegisterClassCheck was added, which allows you to optionally specify a check function. If specified, links will happen only if the check passes. Similar to the (now old) format where link functions returned a success and message variable, the Check function follows the same format. It is not applied to unlink functions because you should always be allowed to unlink.

RegisterClassDelay was also added, which allows you to optionally specify a delay for links to be applied when called from a chip (SF/E2). This can greatly reduce the benefits that can arise from on the fly link manipulation.

RegisterClassStore1 and RegisterclassStore2 can be used to optionally specify a LinkData table. If specified, a table on the entity will be automatically managed to contain the current linkages an entity of Class1 has of Class2. This is intended to make it much cleaner to maintain ordered and unordered links.

ACF.AD2SaveLinks and ACF.AD2LoadLinks makes it easier to interface links with AD2

ACF.UnlinkAll just unlinks all links of a given type; just for convenience.

Added/Current link functions:
ACF.GetLinkDataSafe
ACF.RegisterClassLink
ACF.RegisterClassUnlink
ACF.RegisterClassCheck
ACF.RegisterClassDelay
ACF.RegisterClassStore1
ACF.RegisterClassStore2
ACF.AD2SaveLinks
ACF.AD2LoadLinks
ACF.UnlinkAll

Removed link functions:
RegisterNewLink
GetClassLink
GetClassUnlink

Copy link
Member

@TwistedTail TwistedTail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides avoiding sequential links as a thing, it only needs a few readability changes, along with clarifying some stuff I couldn't really grasp why it was done the way it was.

Data2[Class1] = function(Ent2, Ent1)
return Function(Ent1, Ent2)
end
local LinkData2 = ACF.GetLinkDataSafe(Class1,Class2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this be the same as line 451?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it should be reversed to

local LinkData2 = ACF.GetLinkDataSafe(Class2,Class1)

return ClassLink.Link[Class1][Class2]
--- @param Function fun(Entity1:table, Entity2:table, FromChip:boolean) The checking function defined between an entity of Class1 and an entity of Class2
function ACF.RegisterClassCheck(Class1, Class2, Function)
ACF.GetLinkDataSafe(Class1,Class2).Check = function(Ent1, Ent2, FromChip) return Function(Ent1, Ent2, FromChip) end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability, localize the result from ACF.GetLinkDataSafe and then assign it's Check value.

function ACF.RegisterClassCheck(Class1, Class2, Function)
ACF.GetLinkDataSafe(Class1,Class2).Check = function(Ent1, Ent2, FromChip) return Function(Ent1, Ent2, FromChip) end
if Class1 == Class2 then return end
ACF.GetLinkDataSafe(Class2,Class1).Check = function(Ent2, Ent1, FromChip) return Function(Ent1, Ent2, FromChip) end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability, localize the result from ACF.GetLinkDataSafe and then assign it's Check value.

RegisterNewLink("Unlink", Class1, Class2, Function)
--- @param ChipDelay number If the link happens from the chip, then delay it by this amount
function ACF.RegisterClassDelay(Class1, Class2, ChipDelay)
ACF.GetLinkDataSafe(Class1,Class2).ChipDelay = ChipDelay
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability, localize the result from ACF.GetLinkDataSafe and then assign it's ChipDelay value.

function ACF.RegisterClassDelay(Class1, Class2, ChipDelay)
ACF.GetLinkDataSafe(Class1,Class2).ChipDelay = ChipDelay
if Class1 == Class2 then return end
ACF.GetLinkDataSafe(Class2,Class1).ChipDelay = ChipDelay
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability, localize the result from ACF.GetLinkDataSafe and then assign it's ChipDelay value.

function ACF.GetClassUnlink(Class1, Class2)
if not ClassLink.Unlink[Class1] then return end
function ACF.RegisterClassStore1(Class1, Class2, LinkStore)
ACF.GetLinkDataSafe(Class1,Class2).LinkStore1 = LinkStore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability, same reason as before.

function ACF.RegisterClassStore1(Class1, Class2, LinkStore)
ACF.GetLinkDataSafe(Class1,Class2).LinkStore1 = LinkStore
if Class1 == Class2 then return end
ACF.GetLinkDataSafe(Class2,Class1).LinkStore2 = LinkStore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability, same reason as before.


return ClassLink.Unlink[Class1][Class2]
function ACF.RegisterClassStore2(Class1, Class2, LinkStore)
ACF.GetLinkDataSafe(Class1,Class2).LinkStore2 = LinkStore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability, same reason as before.

function ACF.RegisterClassStore2(Class1, Class2, LinkStore)
ACF.GetLinkDataSafe(Class1,Class2).LinkStore2 = LinkStore
if Class1 == Class2 then return end
ACF.GetLinkDataSafe(Class2,Class1).LinkStore1 = LinkStore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability, same reason as before.


local ClassLink = { }

function ACF.GetLinkDataSafe(Class1, Class2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of naming convention, returning a "Safe" value often means a copy of it. In this case, the actual real value is being returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to GetLinkData since its usage requires it to return a real value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally called it "Safe" since it safely checks if the table exists and creates a new one if it doesn't already exist.

This reverts commit ef731d1.
This reverts commit bdc5c6d.
This reverts commit e91eae9.
- Removes LinkStore variables
- Link information is now stored in the direction registered, but retrieval will try either so to keep the way we link things the same
- Link functions will now be called with an additional `FromChip` parameter which specifies if the linkage comes from a chip.
- Links will now support a `ChipDelay` property which will apply the linkage a set time after the initial call. This should be used with the `RegisterClassCheck` function.
Copy link
Member

@TwistedTail TwistedTail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just minor changes, everything else seems alright.

--- @return LinkData? LinkData The returned link
--- @return boolean Reversed Whether you should reverse your entity arguments when calling with entities
function ACF.GetClassLink(Class1, Class2)
if ClassLink[Class1] ~= nil and ClassLink[Class1][Class2] ~= nil then return ClassLink[Class1][Class2], false end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does ClassLink store any booleans that could end up being false? If not, comparing if X ~= nil is unnecesary, you can just do if X on that case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I hallucinated this not working but it seems it works now. The other ones with ~= nil will be fixed.

local Function = ACF.GetClassLink(self:GetClass(), Class)
local LinkData, Reversed = ACF.GetClassLink(self:GetClass(), Class)

if LinkData == nil then return false, "Links between these two entities are impossible" end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If LinkData never actually receives a false value as a valid result, then you could just change this condition to if not LinkData

elseif self.DefaultLink then
return self:DefaultLink(Target)
if Check then
local result, message = Check(A, B)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent capitalization

local LinkData, Reversed = ACF.GetClassLink(self:GetClass(), Class)
local Function = LinkData.Unlink

if LinkData == nil then return false, "Links between these two entities are impossible" end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If LinkData never actually receives a false value as a valid result, then you could just change this condition to if not LinkData

@TwistedTail TwistedTail merged commit 3bde914 into ACF-Team:dev Aug 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants