feat: added Graph module to Math #3404
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/engine/Math/Graph.ts
Fixed
| const generatedUuid = uuid.replace(/[xy]/g, function (c) { | ||
| const r = (Math.random() * 16) | 0; | ||
| const v = c === 'x' ? r : (r & 0x3) | 0x8; | ||
| return v.toString(16); | ||
| }); |
Check failure
Code scanning / CodeQL
Insecure randomness High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
To fix the problem, we need to replace the use of Math.random() with a cryptographically secure random number generator. In a Node.js environment, we can use the crypto module's randomBytes function to generate secure random values. We will modify the generateUUID method to use crypto.randomBytes instead of Math.random().
| @@ -1,2 +1,3 @@ | ||
| import { Vector } from './vector'; | ||
| import { randomBytes } from 'crypto'; | ||
|
|
||
| @@ -796,3 +797,3 @@ | ||
| const generatedUuid = uuid.replace(/[xy]/g, function (c) { | ||
| const r = (Math.random() * 16) | 0; | ||
| const r = randomBytes(1)[0] % 16; | ||
| const v = c === 'x' ? r : (r & 0x3) | 0x8; |
There was a problem hiding this comment.
Replacing Math.random() wth Ex' Random class
Deploying excaliburjs with
|
| Latest commit: |
e82555a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2dda6921.excaliburjs.pages.dev |
| Branch Preview URL: | https://graph.excaliburjs.pages.dev |
There was a problem hiding this comment.
Looks really awesome, just a couple notes.
Question, do we want to deprecate the pathfinding plugin since we are moving this functionality into core? https://github.com/excaliburjs/excalibur-pathfinding/
| @@ -5,6 +5,79 @@ | |||
| "requires": true, | |||
| "packages": { | |||
| "": { | |||
| "name": "excalibur", | |||
There was a problem hiding this comment.
I think this file will need to be reverted too, makes reference to your local Meet/Excalibur
There was a problem hiding this comment.
might need some guidance on this, i've never touched a package-lock.json
it's usually managed by node, right?
There was a problem hiding this comment.
i've deleted my package-lock and redid a npm install and it keeps coming up with local references in my package-lock...
@eonarheim
| Edges connect nodes and can have properties: | ||
|
|
||
| weight: Numeric value representing distance or cost (default: 0) | ||
| directed: Whether the edge is one-way or bidirectional (default: true) |
There was a problem hiding this comment.
I'm curious if directed should default to false? No strong data, but I generally use bidirectional edges when I'm building nav mesh type situations.
Also is the direction A->B in .addEdge(nodeA, nodeB, ...)?
There was a problem hiding this comment.
changing default is easy, and i can do some parameter renaming to accommodate direction
| * Alias for a node in a graph, representing a vertex in a geometric context. | ||
| * @template T The type of data stored in this vertex. | ||
| */ | ||
| export type Vertex<T> = Node<T>; |
There was a problem hiding this comment.
Should we stick to either node or vertex? I'm a little worried the synonym could be confusing.
There was a problem hiding this comment.
It seems there are duplicate methods with different names
There was a problem hiding this comment.
I would actually prefer to keep it to just nodes, and remove vertex
|
@eonarheim i'm considering modifying how the positions work, and use it to dictate edge weight |
|
@eonarheim we should deprecate the pathfinding plugin in, but I would think we should recreate the sample application using the core native Graph library before deprecating |
Open to feedback on the API here.
Features added, Graphs, Nodes, Edges, Vertices...
Pathfinding and graph traversal functions
QOL methods for using graphs...
Special reqeust for feedback on the Astar vs Dijkstra's pathfinding.... the API is subtle different where Astar uses spatial PositionNodes and Dijkstra's uses weighted edges.
Open for ideas on this.
3 Files added