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

Merge #72, #59, #45, Refactoring and Theme System #79

Merged
merged 53 commits into from
Jul 21, 2024
Merged

Merge #72, #59, #45, Refactoring and Theme System #79

merged 53 commits into from
Jul 21, 2024

Conversation

VitoBarra
Copy link
Contributor

@VitoBarra VitoBarra commented May 2, 2023

Merged #45. Made some alterations to to fix bugs, including making it work with zoom, making it work with small .bin files, and changing the IO to busses for ease of use.
Merged #59.
Merged #72. Fixed 2 minor bugs.

In addition, I have done

  • Major refactoring of the project. (The important part of this PR :P)
  • Added a Theme system for customizable wire and signal colors.
    • Change and Save\load Selected theme Color (Theme System completion)
    • Ability to Save\load Group size
    • Ability to Save\load Signal WireType
  • Enhanced the signal property menu to allow for group size and bus value changes by adding functionality and new keybindings:
    • Q and E: to cycle between default themes.
    • R and F: Change the group size.
    • Save changes made in the GroupProperty menu using Enter and Space.
    • Discard changes made in the GroupProperty menu using Exit.
    • Use up arrow (+1) and down arrow (-1) to adjust integer input fields in the GroupProperty menu.

QOL of the main menu:
now in the load project menu you can

  • Delete a project
  • Renominate a project
  • Duplicate a project

Fix:

notes

To implement these features, the format of save files needed to be reworked, for now the format is not very efficient, more data than needed are stored for each subchip, this maybe will be fixed in the next release

Credits haven't been updated

HannesGitH and others added 30 commits December 11, 2022 10:04
i only did this becouse i dontz know unity very well and couldnt get loading and storing buttons on the eeprom itself to work..
Text is now a little smaller, but it should still be readable on most monitors. 16 bit busses now always stay completely within the text box and look nice and pretty
fixed buggy zoom
fixed small bin files shrinking contents
used busses
got rid of Start() because it was awful
simplified code
Copy link

@901238746 901238746 left a comment

Choose a reason for hiding this comment

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

Some of this looks fine but im a begginer. what happened to Palette.asset?

@901238746
Copy link

901238746 commented Jun 16, 2024

I'd love this pull request because Im making a 11x16(11-bit 16 addresses) For my ALU and later I might need to make it 16 by 16 so that it would work with a better version of my ALUs LU(Logic Unit) so it can do more operations and comparisions. I just need more space. I made this in the original and I would hate it if I had to make it again.

Copy link

@901238746 901238746 left a comment

Choose a reason for hiding this comment

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

This is fine

@AOx0 AOx0 linked an issue Jun 23, 2024 that may be closed by this pull request
VitoBarra and others added 2 commits June 25, 2024 01:01
* WireLayou Merged with chip

* Reimplemented Compatibility loop

* Theme system completed

* Group Signal Completation
@VitoBarra
Copy link
Contributor Author

This should be good to go, some parts of the main menu are not very nice but they work

@VitoBarra VitoBarra requested a review from AOx0 July 8, 2024 17:01
Assets/Scripts/UI/LoadProjectMenu.cs Outdated Show resolved Hide resolved
@AOx0
Copy link
Collaborator

AOx0 commented Jul 8, 2024

Hey, I've been trying the PR, seems good in the Unity Editor, so far.
However, when producing a release build, the input system just stops working with pins, so wires don't work nor the activation/deactivation of the pins signal.

I suspect it's an issue with the new Unity Input system. I can reproduce the issue in the Unity Editor when changing the Event System components to the new suggested Input System UI Input Module.

My next thing on the list is to build the main branch's version and see if the issue is present there too, so I can discard the source code’s fault. Otherwise, if the main branch does work correctly, maybe it would be necessary to git bisect and revert whatever the change caused the button's fault.

I don’t know if you are noticing the same on your end? It may be an issue with Unity in Linux.

@VitoBarra
Copy link
Contributor Author

VitoBarra commented Jul 8, 2024

It happens on my machine (Windows) as well.
I will work on it, but after a first investigation, I found out that issues arise when the SebInput Module is used. This module depends on the new input system, so probably needs to be updated or not used, as in the case of the signal handler that works fine.
I added a package (in the 3rdparty folder) that should be useful for debugging this problem.

@VitoBarra VitoBarra requested a review from AOx0 July 13, 2024 23:10
@AOx0
Copy link
Collaborator

AOx0 commented Jul 16, 2024

Thank you as always Vito!

Just one little nitpick, I don't know if this is intentional, but the changing of pin colors does not display the correct color to be placed. i.e. It displays red but when placed the pin is green.

I'm willing to merge as-is and fix later tho, so I'll be just waiting for your confirmation whether to commit the merge or not.

@VitoBarra
Copy link
Contributor Author

VitoBarra commented Jul 16, 2024

I'm not sure that I understand the concern, but it is intentional that selecting the color for signals (input/output chip pins) and cables requires separate actions.
With this said the PR should be ready c:

@AOx0
Copy link
Collaborator

AOx0 commented Jul 21, 2024

Thank you Vito :D

@AOx0 AOx0 merged commit ae026c6 into DigitalLogicSimCommunity:main Jul 21, 2024
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.

Is #79 ever gonna get approved Some features from the original are not in this version
7 participants