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

First automation attempt, fixes signedness issues and adds missing builds #44

Merged
merged 3 commits into from
Feb 23, 2019

Conversation

Marlamin
Copy link
Collaborator

I built some scripts that will hopefully eventually be able to do automated merges of new versions as soon as they're imported on my site (and exes are dumpable). Whether or not this will push to master or creates a new pull request/branch per build is something I'm still thinking about and is definitely up for discussion!

This is the first run of the merger merging all the versions in this repo (now also automated) with master. Even though the amount of changes is pretty large (99% build additions to build list) I really would appreciate if another person looked this over.

The signedness changes here and there are intended, they are the result of us moving signedness to version definitions instead of column definitions. In said move all the columns became int whether or not they were signed and haven't been fixed due to nothing being re-merged (until now).

Note that due to missing/or incorrect (I'm looking at you 7.0.3) patterns in DBDefsDumper, this script will only automatically merge builds after 25902 to play it safe. When patterns in Dumper improve, it is trivial to lower this build limit.

Again, would appreciate another check (might I suggest the raw diff) before merging this into master!

@barncastle
Copy link
Collaborator

Nice work! A brief glance over and this appears to be fine, I think I only saw two new definitions the rest are just sign changes and missing builds.

The only suspect changes I saw were:

  • GameObjectDisplayInfoXSoundKit's GameObjectDisplayInfoID which went from unsigned to signed, being a relational field I'd expect it to be unsigned.
  • CreatureDisplayInfoTrn's FinishVisualKitID which also went from unsigned to signed, I would've assumed that Start and Finish VisualKitID would be the same signedness.

@Marlamin
Copy link
Collaborator Author

That's mildly suspect, yeah. Other version definitions of those files agree with the new signedness though so not sure if those are wrong. If they are, stuff is being dumped wrongly for other version definitions as well. Will need to double check with meta.

@funjoker
Copy link
Contributor

#36 i adress this

@Marlamin
Copy link
Collaborator Author

Is that exactly the same case with those specific fields? If so, I guess this might be up to the reading implementation to deal with as I'm not sure this is within the scope of the project (or at least right now).

@Marlamin
Copy link
Collaborator Author

f075e7f is also fully automated! Haven't scheduled it yet to run after new builds, but will do when everything has been tested a bit more.

@Marlamin
Copy link
Collaborator Author

Marlamin commented Feb 23, 2019

What @funjoker mentioned does seem to be the case here, but I'm leaving that as a separate issue for now. Everyone think this is good enough to go into master?

EDIT: Got approval via other channels, merging!

@Marlamin Marlamin merged commit 94c4319 into master Feb 23, 2019
@Marlamin Marlamin deleted the automation branch February 23, 2019 15:26
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.

3 participants