-
Notifications
You must be signed in to change notification settings - Fork 77
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
World rendering improvements #408
Changes from all commits
72872dc
dbba29f
bba9943
81f3993
8d671e1
457608c
ed75585
cfb8843
39b0849
cdde86d
efdc802
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,12 @@ const THREE = require('three') | |
const path = require('path') | ||
|
||
const textureCache = {} | ||
// todo not ideal, export different functions for browser and node | ||
function loadTexture (texture, cb) { | ||
if (process.platform === 'browser') { | ||
return require('./utils.web').loadTexture(texture, cb) | ||
} | ||
|
||
if (textureCache[texture]) { | ||
cb(textureCache[texture]) | ||
} else { | ||
|
@@ -22,6 +27,9 @@ function loadTexture (texture, cb) { | |
} | ||
|
||
function loadJSON (json, cb) { | ||
if (process.platform === 'browser') { | ||
return require('./utils.web').loadJSON(json, cb) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this needed? didn't it work before? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As said in the body I moved to esbuild in my p viewer setup so I guess that's why I added it. If this is the only thing that blocks merging the pr and you don't want to see it you can remove it (feel free to do it since you should have push access) |
||
} | ||
cb(require(path.resolve(__dirname, '../../public/' + json))) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,10 +12,14 @@ function mod (x, n) { | |
class WorldRenderer { | ||
constructor (scene, numWorkers = 4) { | ||
this.sectionMeshs = {} | ||
this.active = false | ||
this.version = undefined | ||
this.scene = scene | ||
this.loadedChunks = {} | ||
this.sectionsOutstanding = new Set() | ||
this.renderUpdateEmitter = new EventEmitter() | ||
this.blockStatesData = undefined | ||
this.texturesDataUrl = undefined | ||
|
||
this.material = new THREE.MeshLambertMaterial({ vertexColors: true, transparent: true, alphaTest: 0.1 }) | ||
|
||
|
@@ -60,23 +64,43 @@ class WorldRenderer { | |
} | ||
} | ||
|
||
setVersion (version) { | ||
resetWorld () { | ||
this.active = false | ||
for (const mesh of Object.values(this.sectionMeshs)) { | ||
this.scene.remove(mesh) | ||
} | ||
this.sectionMeshs = {} | ||
for (const worker of this.workers) { | ||
worker.postMessage({ type: 'reset' }) | ||
} | ||
} | ||
|
||
setVersion (version) { | ||
this.version = version | ||
this.resetWorld() | ||
this.active = true | ||
for (const worker of this.workers) { | ||
worker.postMessage({ type: 'version', version }) | ||
} | ||
|
||
loadTexture(`textures/${version}.png`, texture => { | ||
this.updateTexturesData() | ||
} | ||
|
||
updateTexturesData () { | ||
loadTexture(this.texturesDataUrl || `textures/${this.version}.png`, texture => { | ||
texture.magFilter = THREE.NearestFilter | ||
texture.minFilter = THREE.NearestFilter | ||
texture.flipY = false | ||
this.material.map = texture | ||
}) | ||
|
||
loadJSON(`blocksStates/${version}.json`, blockStates => { | ||
const loadBlockStates = () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this needed? didn't it work before? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still works, but now you can override logic of getting block states via exposed property so you can skip network request. Ideally it should also set to this property if wasn't set before after request, fixed in my version but not here but that's not a big issue imo. Overall it's not a bug fix, it's a customisation improvement |
||
return new Promise(resolve => { | ||
if (this.blockStatesData) return resolve(this.blockStatesData) | ||
return loadJSON(`blocksStates/${this.version}.json`, resolve) | ||
}) | ||
} | ||
loadBlockStates().then((blockStates) => { | ||
for (const worker of this.workers) { | ||
worker.postMessage({ type: 'blockStates', json: blockStates }) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this needed? didn't it work before?