Improvements to the README for contributors

[HRahikainen]

Hello,

I have previously used CedarLogic for university coursework and stumbled upon it again looking for C++ practice projects. I managed to build the project and run the installer but noticed that there aren't many instructions for newcomers.

I think it would be nice to have short instructions on how to work with the project as a developer:

• running unit tests
• debug build/viewing logs
• what branching model is used when contributing

I myself haven't worked on Windows and VisualStudio solutions but I am familiar with C and C++ and would be glad to help.

[JoeHCQ1]

Hi @HRahikainen,

Yes, more helpful documentation especially for contributors is always welcome.

Branching model is the GitHub flow, I'd link to it but am using the GitHun mobile app at the moment so I'll just say the name and trust anyone who wants to know more can Google it.

There aren't any unit tests. There should be, and there are in my fork. Unfortunately, I haven't worked on this for a couple months and those unit tests are on a rebuilt core engine that needs to be integrated with the current GUI code.

I'll post more info on that for you if you're interested in it once I'm back at my computer.

[HRahikainen]

@JoeHCQ1 Thanks for the info! I'd be happy to hear more about the setup and how I might help. I was looking at the codebase and trying to figure out where to start considering the v2.4 goals listed here in the issues. Definitely need to find a way to bring tests for current code in order to be confident in refactoring.

[JoeHCQ1]

@HRahikainen as I never got on my computer yesterday (demoed and started remodeling my master bathroom) here is the link to my fork where I've done a lot of the latest development: https://github.com/JoeHCQ1/CedarLogic. I am using a fork so I have full admin to add and test GitHub CI/CD pipelines (though I've not actually started that yet)

Here are the open questions and chief challenges:

1. The latest code is very unstable. It crashed in under a minute of me just testing some wire behavior and friends who use it in university tell me they save every other second just about to avoid losing work.
2. The memory model they use especially when resolving wire values (which may be driven by any number of gates) is something of a linked list that is only searched to about 4 levels of indirection.
3. There is a naive use of shared pointers that pretends they can prevent memory leaks rather than using them for their semantic purpose, to show shared ownership.
4. All of those led me to try building the logic engine from scratch rather than refactor the existing code. I do not know if that was the right choice, large changes often fail and at least fail to deliver value as fast as small ones, but there's so much shared state in the existing code base rather than shared values, and separated concerns that unit testing would be difficult to add, since you'd have to mock out lots more than if more logic was pure functions. And as you rightly note, without unit tests it is difficult to refactor. Anyway, my fork had the new core, which should be integratabtle with the current gui and the logic core has nearly comprehensive unit tests. If you look at the network code, I left comments for someone else who was going to implement it, if you can understand what I was getting at, go ahead and try implementing that class. I can point you at suggested algorithms if you'd like.
5. Long term, I doubt we should keep the GUI at least in C++. I've looked at a few other options, one that I'm favoring is to make a c-binding for the new logic core and rebuild the GUI in one of the leading Python frameworks and call into the C/C++ for the speed-critical functions. I can explain this in more detail if you'd like, but the short summary reason why is that C++ ecosystem involves a lot of waste that other languages do not require (notably manual dependency management) and there are few GUI options. Especially when I consider the work involved in supporting cross-platform GUIs, I think it'd be most efficient long term to move the GUI out of C++ or possibly into IAmGUI. That could also work.

All that to say, the tickets may not be fully up-to-date, I've not touched this in the couple months since spring started, but my fork has the latest code iff it makes sense to use it, it is a large all-at-once change, so the new logic core might best be used as a idea to slowly move the current code towards rather than try to actually swap it in. On the other hand, the latest code is so buggy that I'm not sure there's significant value preserving what's there. Also, we could go even further towards big changes and try building the GUI new out of Python (I can find the framework that looked best but no hard reqs there) and avoid integrating with code that held a very different design philosophy.

I wrote more about my design approach in the docs/ in my fork.

Also, your comment about unit tests made me very happy, let me know if you want a modeling and simulation job in C++, C and Python. We could use more people who care about testing. It's military related in the U.S. so U.S. citizenship required.

I should have time to try and update tickets and documentation this afternoon.

[HRahikainen]

@JoeHCQ1

1. I'm still not familiar enough with the architecture to say if the new core is the way to go, but where would you prefer the work to be done, your fork or some experimental branch based on your work on the upstream repo?
2. I definitely like the simulator being a separate library and your fork's cmake build seems much nicer to work with than VStudio solution
3. Cross-platform GUI options (C++, Python, Web...) are definitely a bigger discussion :)

If we can update the tickets and decide on a workflow for changes (big or small), then the work and discussion can continue on specific tickets?

[HRahikainen]

@JoeHCQ1 Just as an update, I've been busy this week but I started looking into isolated components such as the XMLParser, which I can write tests for without many dependencies.

[JoeHCQ1]

Going to work on this one now to make sure each item you named is included in the dev-v2.4 branch readme and then close it.

note: this won't help newcomers since they go to the master branch by default. We need to merge dev into master and go back to the GitHub Flow then the master branch will have the changes.

[JoeHCQ1]

The branching model is in the Contributing document, and the rest is in the Building document, all in branch dev-v2.4.

[JoeHCQ1]

@HRahikainen , do you want to review the docs in the dev-v2.4 branch and let me know if you see anything else that's missing?

The Linux build instructions are not quite working. @ArenM if you can improve those that'd be great!

[ArenM]

Linux builds is the next thing I plan to work on :)

btw, another option for branching would be to set dev-v2.4 as the default branch until it gets merged into master

[JoeHCQ1]

btw, another option for branching would be to set dev-v2.4 as the default branch until it gets merged into master

Yeah, I'd do that except I'm lacking the admin access to the repo. I had it, but then didn't do anything on CedarLogic for long enough I lost the admin during a permissions review. I should ask for it again. I'd probably also update us to just run off of main instead of master.

[HRahikainen]

@HRahikainen , do you want to review the docs in the dev-v2.4 branch and let me know if you see anything else that's missing?

The Linux build instructions are not quite working. @ArenM if you can improve those that'd be great!

Thanks for the heads up, I'll pick up testing the Linux build probably this week and report back!

[HRahikainen]

@JoeHCQ1 Looking at the docs in dev branch, the main README's link to the Building.md is broken (has a relative path of ../docs although should be ./docs/...). Other than that the docs are going in the right direction and hopefully after Linux support is tested we can give developer notes besides Visual Studio too.

Following the build instructions in the @ArenM Linux support PR-branch, I also encountered this missing io.h (CedarLogic/build/_deps/wxwidgets_fetch-src/include/wx/wxcrtbase.h:41:14) that seems to be included inside these ifdefs:

if defined(__WINDOWS__)

#include <io.h>

endif

Any idea where __WINDOWS__ gets defined in the build?

[JoeHCQ1]

Ah, no I don't, but I bet we could make that a custom flag if nothing else that we can set via CMAKE. That's probably reinventing the wheel though. There is probably a "normal" way to do this, just need to find it. I can look when I get a bit of time.

[ArenM]

It looks like __WINDOWS__ is defined by the wxWidgets build system based on __WXMSW__ which is defined in CMakeLists.txt, my Linux support branch checks the platform before adding that option which prevents those errors. If I add that symbol back I get the same errors. I'd recommend using my Linux branch for testing since it fixes a handful of things like that.