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

Use Semver in Dependency Constants #419

Closed
wants to merge 25 commits into from

Conversation

uniboi
Copy link
Contributor

@uniboi uniboi commented Feb 15, 2023

If a mod for a dependency constant is loaded, this will create a sqvm constant that's a SQTable holding the dependency's major, minor and patch version as integer. All other subversions are omitted.
If a dependency is not loaded, the constant will be 0 instead.

This does not break compatability with old versions since a table is a truthy value like 1, as it is now.

This allows modders to compile code for ranges of mod versions.

For example:

#if HAS_UEBERBLICK
// This compiles only if the dependency is loaded. No matter the version of the dependency
#endif

#if HAS_UEBERBLICK && HAS_UEBERBLICK.major == 1
// This compiles ONLY if the dependency version is 1.x.x
#endif

#if HAS_UEBEBLICK && HAS_UEBERBLICK.major == 1 && HAS_UEBERBLICK.minor >= 2 && HAS_UEBERBLICK.minor <= 5
// This compiles only if the dependency's version is between 1.2.x - 1.5.x
#endif

Invalid version numbers are represented as -1.

  • "1.2.test"
    major: 1
    minor: 2
    patch: -1

  • "1."
    major: 1
    minor: -1
    patch: -1

  • "heheheha"
    major: -1
    minor: -1
    patch: -1

etc.

@GeckoEidechse
Copy link
Member

I assume v1.12.1-rc would then be:

major: 1
minor: 12
patch: 1

?

@uniboi
Copy link
Contributor Author

uniboi commented Feb 19, 2023

v1.12.1-rc would be

mj: -1
mn: 12
pt: -1

since I did not bother writing an semver parser because
a) I have never seen a mod with a leading v in the version
b) I don't think anyone is playing on release candidates

@GeckoEidechse
Copy link
Member

v1.12.1-rc would be

mj: -1
mn: 12
pt: -1

since I did not bother writing an semver parser because
a) I have never seen a mod with a leading v in the version

the leading v was a force of habit, forgot that it's not in the mods version

b) I don't think anyone is playing on release candidates

sadly cause we really need more playtesters x_x

@uniboi
Copy link
Contributor Author

uniboi commented Feb 19, 2023

I could just terminate on a -, I guess. parsing rc subversion doesn't make sense imo

@GeckoEidechse
Copy link
Member

I could just terminate on a -, I guess. parsing rc subversion doesn't make sense imo

Yeah that's fine. Was just wondering whether it would be a -1 or whether the -rcX is stripped and turned into just the 1 for patch ^^

@uniboi
Copy link
Contributor Author

uniboi commented Feb 21, 2023

Ignores now anything after a -

@BobTheBob9
Copy link
Member

genuinely surprised any kind of table stuff works in #ifs?

@uniboi
Copy link
Contributor Author

uniboi commented Feb 23, 2023

Conditions for #if are normal squirrel expressions, they just need to be able to be evaluated at compile time (ie be constant).

Even stuff like this works #if { k = 1 }.k == int(CLIENT), even if unnecessary

@uniboi

This comment was marked as outdated.

@BobTheBob9 BobTheBob9 requested a review from a team March 10, 2023 23:51
@uniboi

This comment was marked as outdated.

@uniboi
Copy link
Contributor Author

uniboi commented Mar 11, 2023

fixed again.
dep_test.zip
To test: Install both mods in the zip. In the mp lobby, the globals a = 1, b = 2 and c = 3 should be defined while d = 4 isn't.

@EladNLG
Copy link
Member

EladNLG commented Mar 12, 2023

Small request:

Make the version available as a string as well :)

@uniboi
Copy link
Contributor Author

uniboi commented Mar 12, 2023

Make the version available as a string as well :)

you can now do checks like #if CONSTANT.version == "1.2.3" #elseif CONSTANT.version == "1.2.3-rc1" #endif

Copy link
Contributor

@ScureX ScureX left a comment

Choose a reason for hiding this comment

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

very good i like so hot and sexy mmhmmm nice code yes

Copy link
Member

@catornot catornot left a comment

Choose a reason for hiding this comment

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

tested the pr; works well except it crashed once ( used the last ci build btw )

used a script command to test this : script printt(HAS_DEP_TEST_2.version, "major:",HAS_DEP_TEST_2.major,"minor:",HAS_DEP_TEST_2.major,"patch:",HAS_DEP_TEST_2.patch)

with dep_test_2

1.2.3 major: 1 minor: 1 patch: 3

without dep_test_2

[NORTHSTAR] [info] sq_compilebuffer returned SQRESULT_NULL
[NORTHSTAR] [info] sq_call returned SQRESULT_ERROR

with rc for patch

1.2.3-rc major: 1 minor: 1 patch: 3

another note it also works by just using #if DEP :)

nslog2023-03-20 19-28-52.txt

@uniboi uniboi requested a review from pg9182 March 26, 2023 15:34
@uniboi
Copy link
Contributor Author

uniboi commented Mar 26, 2023

It looks like the game crashes when accessing a dependency constant table and then recompiling scripts

@pg9182 pg9182 removed the request for review from a team April 18, 2023 02:16
@pg9182 pg9182 added the needs code review Changes from PR still need to be reviewed in code label Apr 18, 2023
@ASpoonPlaysGames ASpoonPlaysGames added the waiting on changes by author Waiting on PR author to implement the suggested changes label Aug 25, 2023
@ASpoonPlaysGames
Copy link
Contributor

@uniboi is this still going to be worked on? Or has the crashing discouraged you?

@ASpoonPlaysGames ASpoonPlaysGames added the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Aug 25, 2023
@uniboi
Copy link
Contributor Author

uniboi commented Aug 26, 2023

I think it might just be a refcount issue, I haven't tested any further though.
Might try soon:tm:

@ASpoonPlaysGames ASpoonPlaysGames added merge conflicts Blocked by merge conflicts, waiting on the author to resolve and removed needs code review Changes from PR still need to be reviewed in code merge conflicts Blocked by merge conflicts, waiting on the author to resolve labels Sep 2, 2023
@GeckoEidechse
Copy link
Member

@uniboi heads-up that there are merge conflicts here ^^

@uniboi
Copy link
Contributor Author

uniboi commented Apr 1, 2024

fixed in #685

@uniboi uniboi closed this Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge conflicts Blocked by merge conflicts, waiting on the author to resolve waiting on changes by author Waiting on PR author to implement the suggested changes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants