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

Ignore new common.j natives if game version too low #645

Closed
Frotty opened this issue Apr 30, 2018 · 18 comments
Closed

Ignore new common.j natives if game version too low #645

Frotty opened this issue Apr 30, 2018 · 18 comments
Assignees

Comments

@Frotty
Copy link
Member

Frotty commented Apr 30, 2018

Since we want to ship the latest jass API with wurst, but keep backwards compat,
the new natives should simply be ignored so that everyone can use the same standard library.

@peq
Copy link
Collaborator

peq commented Apr 30, 2018

Hmm, shouldn't this already work as we remove all unused functions?

@karlek
Copy link
Contributor

karlek commented May 9, 2018

I couldn't compile my map as the Blz* natives were missing from common.j, so it's not working atm

@Frotty
Copy link
Member Author

Frotty commented May 9, 2018

@karlek If you want to compile pre 1.29 , you need the stdlib branch pre 1.29 which doesn't contains the extension functions etc.

I guess we need the setup tool now to offer a pre-1.29 setup

@karlek
Copy link
Contributor

karlek commented May 10, 2018

Good to know!

Yeah, I don't think the mapping community in general has the git skill to change branches freely.

@PhoenixZeng
Copy link

yes.
a other 1.29 set is the JASS_MAX_ARRAY_SIZE = 32768 at 1.29
I use the standard library very carefully to make sure that my map can run on 1.27. But there is nothing I can do about this array size.

@Frotty
Copy link
Member Author

Frotty commented May 13, 2018

@PhoenixZeng we don't use JASS_MAX_ARRAY_SIZE anywhere.
What exactly is your problem?

@PhoenixZeng
Copy link

PhoenixZeng commented May 14, 2018

@Frotty in compiled.j.txt
I can find such code many times
and it looks like will report a error
when the ForGroupCallback_maxIndex greater than 8192

	if ForGroupCallback_firstFree == 0 then
		if ForGroupCallback_maxIndex < 32768 then
			set ForGroupCallback_maxIndex = ForGroupCallback_maxIndex + 1
			set this_21 = ForGroupCallback_maxIndex
			set ForGroupCallback_typeId[this_21] = 701
		else
			set wurst_stack[wurst_stack_depth] = ""
			set wurst_stack_depth = wurst_stack_depth + 1
			call error("Out of memory: Could not create Closure.")
			set this_21 = 0
		endif

@Frotty
Copy link
Member Author

Frotty commented May 14, 2018

Yes, but that isn't related to this ticket, which is taking about common.j/blizzard.j

@PhoenixZeng
Copy link

PhoenixZeng commented May 14, 2018

emm.
i think the 32768 should read the common.j - JASS_MAX_ARRAY_SIZE ,but from the Compiler (or other way)
and the common.j/blizzard.j always base on the game version ( or setting when WurstSetup build a project )

@Frotty
Copy link
Member Author

Frotty commented May 14, 2018

Yes, if we want to support pre 1.29 development fully, we probably need some fallback functionality.
However, if you reach the array limit, you need to fix your code anyway :)

@daffaabiyyu05
Copy link

I doubt it's common to hit 8192 array limit, considering that in it's own is a pretty high number.

@Cokemonkey11
Copy link
Collaborator

Code that doesn't behave well (leaks class instances) will easily hit the array limit.

@PhoenixZeng
Copy link

PhoenixZeng commented Jul 12, 2018

@DaffaMage linkedlist-> LLEntry may hit 8192 in a large map. And interface or class which have too many subclasses will easily hit the array limit

@daffaabiyyu05
Copy link

I see then, my apologies.

@Frotty Frotty self-assigned this Aug 2, 2018
@Frotty
Copy link
Member Author

Frotty commented Aug 2, 2018

Alright so to summarize we want to offer a build flag for pre 1.29 to:

  • Use old common.j/blizzard,j
  • Use old Array size limit
  • Use compatible standard library branch

@PhoenixZeng
Copy link

PhoenixZeng commented Nov 8, 2018

Use compatible standard library branch

It's not necessary.Because the compiler does not pour unused standard library functions.
If old branches are used, many new functions (not native function) will be difficult to handle and maintenance.

maybe can make a annotation @since(1.29)
if the version pre 1.29 , the plugin can error the used code
the hashtable is the same @since(1.24)
Although no one is using pre 1.24 now ,i think
@Frotty

@peq
Copy link
Collaborator

peq commented Nov 8, 2018

If we compile with an old common.j the current version of the standard library will probably give some errors since it uses some new natives.

Why can't we just compile with the new common.j and use pjass to check the final war3map.j against the target version common.j?

And for array size we can just add an option.

@Frotty
Copy link
Member Author

Frotty commented Jan 3, 2020

Yes, I think the project's common.j Blizzard.j aren't passed to Pjass. Other than that it should be working.

@Frotty Frotty closed this as completed in 9f6a733 Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants