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

If you do it quick'n'dirty, your code quality will suffer! #133

Open
3 tasks
Ismoh opened this issue Aug 17, 2023 · 15 comments
Open
3 tasks

If you do it quick'n'dirty, your code quality will suffer! #133

Ismoh opened this issue Aug 17, 2023 · 15 comments
Assignees
Labels
documentation Improvements or additions to documentation rework Predicts breaking changes. This will increase MAJOR version number, when merged into develop.

Comments

@Ismoh
Copy link
Owner

Ismoh commented Aug 17, 2023

Yoo, dependency hell led me to use lua globals. Guess what?! Performance sucks so much!

I try to do a quick test comparing performance and continue on reworling code, if performance increases.

My tasks

@Ismoh Ismoh added documentation Improvements or additions to documentation rework Predicts breaking changes. This will increase MAJOR version number, when merged into develop. labels Aug 17, 2023
@Ismoh Ismoh added this to the Synchronisation milestone Aug 17, 2023
@Ismoh Ismoh self-assigned this Aug 17, 2023
@Ismoh Ismoh added this to NoitaMP Aug 17, 2023
@Ismoh
Copy link
Owner Author

Ismoh commented Aug 17, 2023

Create the branch based on #67

@Ismoh Ismoh moved this to 🏗 In progress in NoitaMP Sep 13, 2023
@Ismoh
Copy link
Owner Author

Ismoh commented Oct 17, 2023

LuaJIT profiler works!

It's findings:
from frame 2060: 73% | local encodedBase64 = self.base64.encode(self.np.SerializeEntity(entityId))

but still as you guys mentioned:
67% | local exists = os.rename(absolutePath, absolutePath) and true or false
it all the time above 60%

@Ismoh
Copy link
Owner Author

Ismoh commented Oct 17, 2023

Fix constructor dependency injection!

Worth testing passing dependencies as parameters on functions?

Best would be to have locals in init.lua and then passing those to classes, like: class:new() then class:requireDependencies(...) per class. When there is any foo is nil, then extend function above. Otherwise it will be p.i.t.a. maintaining it..

@Ismoh
Copy link
Owner Author

Ismoh commented Oct 30, 2023

Get rid of EntityCache?! It was used in the past for old serialization?!

Nope! It is also used to check, if an entity somehow changed and needs to be synced. But worth improving it. Gut feeling tells me, cache clean up, isn't 100% reliable.

@Ismoh
Copy link
Owner Author

Ismoh commented Oct 30, 2023

Debug setting for multiple instances to avoid unecessary resource/performance usage. Save pid in file. New settings file.

Edit: Double checked yesterday, looks fine for now!

@Ismoh
Copy link
Owner Author

Ismoh commented Nov 4, 2023

Chop of NULL byte from NoitaPatcher serialized binary string, when adding to VSC, like NoitaPatcherUtils:prepareForVSC(binaryString), which returns the modified string. Otherwise we cannot store it in VSC, but we need the NULL byte for Deserialization.

@Ismoh
Copy link
Owner Author

Ismoh commented Nov 4, 2023

Chop of NULL byte from NoitaPatcher serialized binary string, when adding to VSC, like NoitaPatcherUtils:prepareForVSC(binaryString), which returns the modified string. Otherwise we cannot store it in VSC, but we need the NULL byte for Deserialization.

I am not able to do this and won't do it. If there is any contributer interested in go for it, but it's waste of time. Base64_ffi is fast enough.

@Ismoh
Copy link
Owner Author

Ismoh commented Nov 29, 2023

Add toggle setting for pprint and pformat. It's executed each frame and is not necessary, when log level is off.

@Ismoh
Copy link
Owner Author

Ismoh commented Nov 29, 2023

Add checks before sending wrong network data. Some mandatory tables are nil now and then. Leads to wrong assumptions.

@Ismoh
Copy link
Owner Author

Ismoh commented Nov 30, 2023

If an entity was serialized and sent to anyone and then it got deserialized, it can happen, that the deserialized entity is the same as on the other peer. Sounds awesome? Yes and no. When Minä got synced, it's a huge problem.

Therefore double check when an entity was deserialized and a similiar one was found, whether nuids are matching, if not check the owner and guid are the same, if not create a new entity and do not reuse existing entities.

@Ismoh
Copy link
Owner Author

Ismoh commented Nov 30, 2023

Chop of NULL byte from NoitaPatcher serialized binary string, when adding to VSC, like NoitaPatcherUtils:prepareForVSC(binaryString), which returns the modified string. Otherwise we cannot store it in VSC, but we need the NULL byte for Deserialization.

I am not able to do this and won't do it. If there is any contributer interested in go for it, but it's waste of time. Base64_ffi is fast enough.

Maybe @aaptel can help me with this? We can have a discord call, when you are free!

@Ismoh
Copy link
Owner Author

Ismoh commented Nov 30, 2023

Wise double checking this?
Assign package.loaded["Classname"] = nil when the class was loaded, to make it free for garbage collections.

@aaptel
Copy link
Collaborator

aaptel commented Nov 30, 2023

Chop of NULL byte from NoitaPatcher serialized binary string, when adding to VSC, like NoitaPatcherUtils:prepareForVSC(binaryString), which returns the modified string. Otherwise we cannot store it in VSC, but we need the NULL byte for Deserialization.

I am not able to do this and won't do it. If there is any contributer interested in go for it, but it's waste of time. Base64_ffi is fast enough.

Maybe @aaptel can help me with this? We can have a discord call, when you are free!

I'm missing a lot of context here. I don't see prepareForVSC in NoitaPatcherUtils.lua

@Ismoh
Copy link
Owner Author

Ismoh commented Dec 5, 2023

Use GitHub Copilot for generating unit tests. Is there a cli copilot version?

YES

@Ismoh
Copy link
Owner Author

Ismoh commented Dec 13, 2023

White- instead of blacklisting entities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation rework Predicts breaking changes. This will increase MAJOR version number, when merged into develop.
Projects
Status: 🏗 In progress
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants