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

Add Queues to E2 File Extension #2763

Merged
merged 7 commits into from
Nov 2, 2023
Merged

Conversation

Denneisk
Copy link
Member

@Denneisk Denneisk commented Sep 25, 2023

Adds queuing to file extension. This means you can do multiple fileLoad, fileWrite, and fileList operations in a single execution without them overwriting each other. This is helpful since the original file system is clunky to use without it. Additionally, this contains many other QoL and fixes.

  • Add queuing for file load operations
  • Added cvar wire_expression2_file_max_queue, default 5, to limit number of queued files
  • Added event fileList(sr) to eventify fileListClk
  • Added event event fileWritten(ss), triggers when file finishes writing to client
  • Added file<Load/Write/List>Queued() to return number of queued elements
  • Added outputs to various functions for debugging
  • Added various exceptions
  • Increased file Load/Write/List op cost to 100 (from 20)
  • Decreased op costs of certain functions
  • Fixes runOnFile triggering when runOnFile is not set
  • Fixes runOnList triggering when runOnFileList is not set
  • Deprecate runOn*- and *Clk-type functions
  • Removed some previously useless code
  • Removed old delay system

Test code (Warning, this does create files in your e2files root folder)

Request for comments:

  • Should op costs be higher? Increase op costs depending on queue size?
  • Should queue be cleared based on a rate instead of instantly?
  • Increased queue size?

Denneisk and others added 3 commits September 10, 2023 21:24
- Add queuing for file load operations
- Added cvar wire_expression2_file_max_queue to limit number of queued files
- Added event fileList(sr) to eventify fileListClk
- Added file<Load/Write/List>Queued() to return number of queued elements
- Added outputs to various functions for debugging
- Added various exceptions
- Increased file Load/Write/List op cost to 100 (from 20)
- Decreased op costs of certain functions
- Fixes runOnFile triggering when runOnFile is not set
- Fixes runOnList triggering when runOnFileList is not set
- Deprecate runOn*- and *Clk-type functions
- Removed some previously useless code
- Removed old delay system
@Denneisk
Copy link
Member Author

Would appreciate reviews and comments.

Comment on lines 295 to 305
for k, v in ipairs(downloads[player]) do
if v.ent == entity then table.remove(downloads, k) end
end

if strlen > 0 then
net.Start("wire_expression2_file_download_chunk")
net.WriteUInt(strlen, 32)
net.WriteData(fdata.data, strlen)
net.Send(ply)
for k, v in ipairs(uploads[player]) do
if v.ent == entity then table.remove(uploads, k) end
end

fdata.data = string.sub( fdata.data, strlen + 1 )
end
for k, v in ipairs(lists[player]) do
if v.ent == entity then table.remove(lists, k) end
end
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 worrying, what happens if the chip is removed during an upload/download? Would another queued chip receive the correct data?

Also these need to be while loops because you are modifying the array that you are iterating.

Copy link
Member Author

Choose a reason for hiding this comment

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

The next chip in the queue would receive a file transfer error because the flag uploading would be unset, but I'll look into avoiding that.

Copy link
Member Author

Choose a reason for hiding this comment

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

This has been mitigated in recent commits.

Optimize some small things
Fixed backwards *Queue functions
Avoid race condition with event fileWritten
Copy link
Contributor

@thegrb93 thegrb93 left a comment

Choose a reason for hiding this comment

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

The code looks fine. It could be even cleaner if downloads, lists, uploads was converted to a single table of actions the player owns and if each action type had a class structure so that code was concentrated into the class table. This is fine for now though

@thegrb93 thegrb93 merged commit 12e109e into wiremod:master Nov 2, 2023
1 check failed
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