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

rewrite arrays to structs #8

Open
turican0 opened this issue Jan 27, 2021 · 10 comments
Open

rewrite arrays to structs #8

turican0 opened this issue Jan 27, 2021 · 10 comments
Labels
documentation Improvements or additions to documentation

Comments

@turican0
Copy link
Collaborator

I'm glad that the project continues and sometimes I help with it (turican0).
Now I'm rewriting the array into structures, this is necessary for further orientation in the code, but it will cause a number of regressions.
When it's all done, I'll start next bug hunting and repair code.

@GrimSqueaker
Copy link
Owner

Hi Tomas,

thanks for your efforts. It is great that you can continue. I would like to make it compile and work on 64bit in the future and the structures are a prerequisite for it. In this area you are the expert.
For all of us this is a hobby, so there is no pressure and only fun. :-)

Regarding bug hunting:
I was a bit too fast to merge your changes and they introduced a regression, see Tim (@thobbsinteractive) and my conversation here: thobbsinteractive#15
To summarize: The fireball sprites are broken when hitting buildings. One of the following commits introduced it:

$ git log 2b098da3^..aac4f686
commit aac4f68699453da60c014428242bf01be87fd7e3
Author: turican0 <[email protected]>
Date:   Tue Jan 26 20:04:27 2021 +0100

    rewrite nest arrays to sctruct

commit 3533ac298561a0fd078c2bbb9e1473666fc0d4db
Author: Tomas Vesely <[email protected]>
Date:   Tue Jan 26 15:27:53 2021 +0100

    next fixes

commit 3de3e51912699518b86acaeec5034e82d6f02669
Author: turican0 <[email protected]>
Date:   Mon Jan 25 18:42:54 2021 +0100

    modify info text

commit 2b098da3fb4b9294456955bcd629368826458451
Author: turican0 <[email protected]>
Date:   Mon Jan 25 18:37:32 2021 +0100

    add informations for next testing

Cheers and thank you,
Sebastian

@turican0
Copy link
Collaborator Author

Hi, thank you for report. I hope yours cooperation on this project. Now I want maximalize rewrite code for better readibility. This phase will be long 1-4 week. During this time, I would only fix bugs that prevented the engine from running. So that my rewriting does not hinder things aka rendering changing, test run on other OS, etc.
I will find errors until after this phase, because it is now faster to look for all errors at once, not each one separately.

I would divide bug hunting on small and big.

Small bug hunting - fix first level, not visible problems - time for this cca 1day(plus find bug fixes)
Big bug hunting - replay all game, every level begin(compare data with dosbox version), test all spells in game(compare data with dosbox version), test all options in game, etc. - time for this cca 30day(plus find bug fixes)

After rewrite structures I make only small testing.
I would like to agree on a Big bug hunting with you.
It is advisable to do this only after maximum code cleanup and rewriting.

(I apologize for my English, I don't speak much :)

Tom.

@GrimSqueaker
Copy link
Owner

Hi Tom,

yes, rewriting for better readability is a really good thing to do.
Until now I have just limited my changes, because I do not know if you for example still need the memory address references in the function and variable names. I guess that this makes it easier for you to compare to the original in dosbox, does it?

I am not sure, if I can help you with the bug hunting, because some areas of the code are still disabled on Linux (they do not compile because of type problems) and I do not know how to compare to the dosbox version. I'll try to get the Linux build as far as possible so that I can support you as good as I can. Please let me know how I can help you.

Cheers,
Sebastian

@turican0
Copy link
Collaborator Author

Hi Sebastian,

Yes at now is easer have memory references in name(or in comment after name?) of functions for compare with dosbox.

For bug hunting I use https://github.com/turican0/dosbox-x-remc2. I modify original DOSBOX-X to extract memory parts at some conditions.
This memory parts I compare with memory parts in remc2 at same conditions and find position of code creating differences(bugs).
After rewriting some structures I will create little manual for this.
If you find non-compilable code in linux parts of code, write it please in new issue. Maybe some will be fixed during rewriting.

By,
Tom

@GrimSqueaker
Copy link
Owner

Hi Tom,

thanks for the explanation. It would be great to have a small manual for the memory comparison.

I think I will deal with all the Linux related compile problems step by step. At the moment I do it like this: Play a level and check the printout (the "FIXME: types @ function"). Fix the types and play again to check that I did not break anything. When starting with the Linux build I did decide to comment it out only for Linux in order to quickly have a working build.
But the "play a level" part take a while. :-)
But I do appreciate if you fix some of them during your rewrites. The GitHub continuous integration will tell you if a pull request breaks the Linux build. That is why I did implement it as a first thing. This way you can work on either system and easily find out if it still works on the other.

Cheers,
Sebastian

@turican0
Copy link
Collaborator Author

Yes, after I rewrite and fix some bugs, I find parts witch FIXME: types, and try fix this code.
Manual to compare with dosbox-x I write, when I do this action.

By,
Tom.

@turican0
Copy link
Collaborator Author

Can you give me the rights to add labels and close issue? (I will not manipulate with git code)
Thank you,
Tom.

@GrimSqueaker
Copy link
Owner

I have added you as a collaborator. I think you got a invitation for this.

@turican0
Copy link
Collaborator Author

turican0 commented Feb 1, 2021

Thank you, but unfortunately I still can't assign labels :(
Tom.

@turican0 turican0 added the documentation Improvements or additions to documentation label Feb 1, 2021
@turican0
Copy link
Collaborator Author

turican0 commented Feb 1, 2021

It already works, I had to confirm the note, I was not on the email in the morning :)

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
Projects
None yet
Development

No branches or pull requests

2 participants