| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Hey, @zees-dev thanks for the submission! Quick note: this edits some sections of emulator.js that are being updated by #1106. I'm not sure which order that @ethanaobrien wants to apply these changes in, but one of us will have to refactor things. Given my change in #1106 migrates a lot of code to external classes, I think it might be this one. Hopefully when the time comes the refactoring won't be too onerous. On to the review: How much of this code did you write vs AI? The PR itself feels very AI-ish (lots of use of emoji) - and while AI assisted code is allowed, "vibe coding" is not. What is the purpose of the findElementId function? Best I can tell, this could have just been a .includes on the element's classList property. You appear to be looking for a class that begins with "b_" but then specify it with the fallback parameter anyway. Why couldn't you use document.getElementsByClassName() or document.querySelectorAll() to select the elements you want to work with by class? @ethanaobrien: In data/src/virtualGamepadEditor.js some icons have been defined in code. I thought about this when I was redo-ing some of the toolbar code - I think moving all icons to either standalone files or as consts in a separate JS file is the best way to go. It makes code more readable to not have to scroll past tons of SVG definitions. Moving the images to their own files does make it a bit easier for devs to customise the emulator with their own icons a bit easier - though it also means we're managing extra files in the package. |
Sorry, something went wrong.
|
Thanks for the quick look/review! AI usage: Review: I havent validated the fallbacks use - In fact i don't think it's needed and will update the code accordingly (thanks for your quick review). Detailed response - How it works At a high level this is a virtual gamepad layout persistence system; it lets users customize touch control positions and saves those customizations across sessions.
Re: findElementId and querySelectorAll Testing: High-level approach: Why this implementation is good? Potential next steps: |
Sorry, something went wrong.
|
@michael-j-green - any updates |
Sorry, something went wrong.
|
@zees-dev: I've requested a review from the repo maintainer. |
Sorry, something went wrong.
| data[i].elem.removeEventListener(data[i].listener, data[i].cb); | ||
| } | ||
| } | ||
| findElementId(classList) { |
There was a problem hiding this comment.
Doesn't this seem redundant given there is already a built in Javascript function to do this?
Sorry, something went wrong.
There was a problem hiding this comment.
Replied in previous comment:
Re: findElementId and querySelectorAll
The layout is stored as { "b_A": {...}, "b_Start": {...} } and persisted to localStorage. To apply saved positions, we need the string key (e.g., "b_A") to do the lookup.
querySelectorAll returns DOM elements, not strings. Even if we selected by [class*="b_"], we'd still need to extract the actual class name to use as the dictionary key. That's what findElementId does.
Sorry, something went wrong.
|
I have been testing this for a bit now; there are some minor issues which need ot be resolved. Also I think we should support orientation based configuration. |
Sorry, something went wrong.
- added jsdocs for class - added jsdocs for all functions in EJS_VirtualGamepadEditor class
- added jsdocs for class - added jsdocs for all functions in EJS_OverlayElement class
- added jsdocs for class - added jsdocs for all functions in EJS_HistoryManager class
- fixed issue with ghost selection area on top of d-pad and joystick
- improved preview
- removed redundant class(es)
- removed white button outlines in editmode; only has dashed outline - virtual gamepad edit mode visual improvements
|
I have now validated that the D-pad seems to be edittable aswell. As for orientation-specific edit controls; im hoping that can be addressed in a future PR (if required). |
Sorry, something went wrong.
…ecture - replace command-style undo/redo with snapshot-based history (past/present/future) - initialize editor state from an initial snapshot and commit changes uniformly after drag/resize/reset/clear - register virtual gamepad controls once in emulator and reuse that registry in editor overlay setup and layout apply - remove generated fallback control ids and persist/edit only explicitly identified controls - streamline pointer interaction handling with Pointer Events first and legacy touch/mouse fallback - normalize virtual gamepad editor toolbar localization keys to plain action labels
|
FYI: not sure if you've done it or not, but there's been a few pushes to main in the last few days, so you may need to check that you don't have any merge conflicts. |
Sorry, something went wrong.
should be rebased on current main |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Description
Implements a visual editor for the virtual gamepad on mobile devices, allowing users to fully customize their control layout.
Closes #588
Features
Technical Implementation
Overlay-Based Architecture
Rather than manipulating the actual gamepad elements directly (which proved to be computationally intensive), this implementation creates an overlay layer during edit mode:
New Files
Modified Files
Key Design Decisions
Screenshot
Usage
Future Improvements
Testing
Tested on:
Breaking Changes
None. Existing virtual gamepad functionality remains unchanged. Custom layouts are opt-in.