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

Pocket #3584

Merged
merged 9 commits into from
Nov 6, 2024
Merged

Pocket #3584

merged 9 commits into from
Nov 6, 2024

Conversation

pvaiko
Copy link
Contributor

@pvaiko pvaiko commented Nov 4, 2024

No description provided.

pvaiko and others added 8 commits November 2, 2024 15:27
Instead of the dz, as it depends on the vehicle's heading.
No matter how much try arround with that, the most elegant solution would be having the distance between our reference waypoint and the pipe of the harvester/combine, be long enought to fit the unloader (in meter, as we don't know what unloader will unload the combine). With that known distance, we coould calculate how far we need to drive back, to then drive into the pocket and get our harvester straight.
Use the front marker to create the helper node
@pvaiko pvaiko requested a review from schwiti6190 November 4, 2024 15:37
@pvaiko pvaiko self-assigned this Nov 4, 2024
---@param ly number y coordinate of the point relative to the node
---@param lz number z coordinate of the point relative to the node
---@return number, number, number
function HelperTerrainNode:localToLocal(node, lx, ly, lz)
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is only working for HelperTerrainNode and not for the base HelperNode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

function HelperNode:init(name, rootNode)
self.node = createTransformGroup(name)
self.rootNode = rootNode
link(self.rootNode, self.node)
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens here if rootNode is nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

-- A helper node that is linked to the terrain root node, and the y coordinate is always
-- the terrain height at the x, z position.
---@class HelperTerrainNode : HelperNode
HelperTerrainNode = CpObject(HelperNode)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if inheritence is the best approach here.
I would prefer something like this:

-- Creates a new helper node linked to the terrain root node.
function HelperNode.createTerrainNode(name )
  return HelperNode(name, g_currentMission.terrainRootNode)
end

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 had something similar first, the reason I don't like it, is that the behavior/interpretation for the y coordinate is different.

---@param y number if the node is linked to the terrain, this is relative to the terrain height at x, z
---@param z number
---@param yRotation number|nil Rotation set only if not nil
function HelperTerrainNode:place(x, y, z, yRotation)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer this functionality in a separate function like so maybe:

---@param yRotation number|nil
---@param yOffset number|nil
function HelperNode:placeTerrain(x, z, yRotation, yOffset)

end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was struggling with this as well, probably we don't even need a generic HelperNode (at least as of now, I'm not aware of a use case), and should only implement a HelperTerrainNode.

-- It wraps the creation of the Giant's engine node.
-- NOTE: it must be destroyed explicitly to avoid node leak.

---@class HelperNode
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a fantastic idea :)
Maybe you got the motivation to add missing commanly used functions, like worldToLocal(), localDirectionToWorld().

@schwiti6190 schwiti6190 self-requested a review November 5, 2024 19:00
@Tensuko Tensuko merged commit 52b4a0a into main Nov 6, 2024
4 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.

3 participants