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

Various Typing Fixes & Client-Side Render Event (Multiple Client Compatible) #428

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

GenerelSchwerz
Copy link

I fixed some typings for Mineflayer, specifically the bot.viewer object.

I also implemented a system that keeps track of the highest FPS of clients currently connected to the viewer. This is used to provide a "render" event that guarantees at least one firing per frame of the fastest client connected.

I chose to poll the FPS of each client every second. That can be tweaked if performance is an issue.

There is unnoticeable impact to performance, both computationally and memory-wise. (It only stores a mapping of socket ids to a number).

In the event that no clients are connected, the event will not fire and the interval that would be firing said event is cleared.

Copy link

socket-security bot commented Feb 26, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@types/three@0.161.2 None +2 1.14 MB types

View full report↗︎

@GenerelSchwerz
Copy link
Author

A demonstration of this technique is shown here.

2024-02-26.00-01-49.mp4

@GenerelSchwerz
Copy link
Author

GenerelSchwerz commented Feb 26, 2024

There appears to be an 1.8.8 error. However, that doesn't appear to be my fault; the error says the test did not find the server (404 error). I will attempt to re-run.

@@ -0,0 +1,3 @@
{
"print.colourScheme": "GitHub"
Copy link
Member

Choose a reason for hiding this comment

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

Please remove

examples/bot.js Outdated
})

bot.once('spawn', () => {
bot.once('spawn', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

That doesn't do anything, please undo

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

const fps = Math.ceil((frameCount / ((currentTime - lastTime) / fpsCheck)))
lastTime = currentTime
frameCount = 0
if (fps != null) socket.emit('renderFPS', { id: socket.id, fps })
Copy link
Member

Choose a reason for hiding this comment

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

Please add to doc + use it in an example to explain what it's good for

Copy link
Author

Choose a reason for hiding this comment

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

I do not know where I would put this server-sided event (renderFPS) firing into docs as there is no mention of server-sided code in the READMEs.

I included documentation on the bot.viewer event firing, however.

@rom1504 rom1504 added this to Needs triage in PR Triage Mar 17, 2024
io.on('connection', (socket) => {
const getRenderInterval = (fps) => setInterval(() => bot.viewer.emit('onRender', fps), 1000 / fps)

const updateListener = ({ id, fps }) => {
Copy link
Member

Choose a reason for hiding this comment

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

please add a comment explaining the intent of this; it's far from obvious

@rom1504 rom1504 moved this from Needs triage to Waiting for user input in PR Triage Mar 17, 2024
@rom1504 rom1504 moved this from Waiting for user input to Almost too old in PR Triage Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
PR Triage
  
Almost too old
Development

Successfully merging this pull request may close these issues.

None yet

2 participants